From 45110a7090fdb3bec82125f3917948058175b65d Mon Sep 17 00:00:00 2001
From: Prateek Saxena <prtksxna@gmail.com>
Date: Sat, 18 Mar 2017 12:08:33 +0530
Subject: [PATCH] Use <button> instead of <div>

* Buttons are semantically better.
* Neither <div> nor <button> elements can have the 'alt' attribute
  so switching to 'title' instead.

Reverts parts of I6ea62124018b1a0ec5110bb500f690cb2368f102.

Bug: T58471
Change-Id: I991b40d3387f8d6dd7cdad8ccc5cc9660b9f139a
---
 resources/mmv/mmv.lightboxinterface.js          |  6 ++----
 resources/mmv/ui/mmv.ui.canvasButtons.js        | 17 +++++++----------
 resources/mmv/ui/mmv.ui.canvasButtons.less      |  1 +
 .../features/support/pages/e2e_test_page.rb     | 10 +++++-----
 tests/qunit/mmv/mmv.lightboxinterface.test.js   |  2 +-
 5 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/resources/mmv/mmv.lightboxinterface.js b/resources/mmv/mmv.lightboxinterface.js
index 9781a690a..f81f60938 100644
--- a/resources/mmv/mmv.lightboxinterface.js
+++ b/resources/mmv/mmv.lightboxinterface.js
@@ -315,11 +315,10 @@
 		var ui = this,
 			tooltipDelay = mw.config.get( 'wgMultimediaViewer' ).tooltipDelay;
 
-		this.$closeButton = $( '<div>' )
+		this.$closeButton = $( '<button>' )
 			.text( ' ' )
 			.addClass( 'mw-mmv-close' )
 			.prop( 'title', mw.message( 'multimediaviewer-close-popup-text' ).text() )
-			.attr( 'alt', mw.message( 'multimediaviewer-close-popup-text' ).text() )
 			.tipsy( {
 				delayIn: tooltipDelay,
 				gravity: this.correctEW( 'ne' )
@@ -331,11 +330,10 @@
 				ui.unattach();
 			} );
 
-		this.$fullscreenButton = $( '<div>' )
+		this.$fullscreenButton = $( '<button>' )
 			.text( ' ' )
 			.addClass( 'mw-mmv-fullscreen' )
 			.prop( 'title', mw.message( 'multimediaviewer-fullscreen-popup-text' ).text() )
-			.attr( 'alt', mw.message( 'multimediaviewer-fullscreen-popup-text' ).text() )
 			.tipsy( {
 				delayIn: tooltipDelay,
 				gravity: this.correctEW( 'ne' )
diff --git a/resources/mmv/ui/mmv.ui.canvasButtons.js b/resources/mmv/ui/mmv.ui.canvasButtons.js
index ff6d64d11..aeb36ccd9 100644
--- a/resources/mmv/ui/mmv.ui.canvasButtons.js
+++ b/resources/mmv/ui/mmv.ui.canvasButtons.js
@@ -38,43 +38,40 @@
 		this.$close = $closeButton;
 		this.$fullscreen = $fullscreenButton;
 
-		this.$reuse = $( '<div>' )
+		this.$reuse = $( '<button>' )
 			.addClass( 'mw-mmv-reuse-button' )
 			.html( '&nbsp;' )
 			.prop( 'title', mw.message( 'multimediaviewer-reuse-link' ).text() )
-			.attr( 'alt', mw.message( 'multimediaviewer-reuse-link' ).text() )
 			.tipsy( {
 				delayIn: tooltipDelay,
 				gravity: this.correctEW( 'se' )
 			} );
 
-		this.$options = $( '<div>' )
+		this.$options = $( '<button>' )
 			.text( ' ' )
 			.prop( 'title', mw.message( 'multimediaviewer-options-tooltip' ).text() )
-			.attr( 'alt', mw.message( 'multimediaviewer-options-tooltip' ).text() )
 			.addClass( 'mw-mmv-options-button' )
 			.tipsy( {
 				delayIn: tooltipDelay,
 				gravity: this.correctEW( 'se' )
 			} );
 
-		this.$download = $( '<div>' )
+		this.$download = $( '<button>' )
 			.addClass( 'mw-mmv-download-button' )
 			.html( '&nbsp;' )
 			.prop( 'title', mw.message( 'multimediaviewer-download-link' ).text() )
-			.attr( 'alt', mw.message( 'multimediaviewer-download-link' ).text() )
 			.tipsy( {
 				delayIn: tooltipDelay,
 				gravity: this.correctEW( 'se' )
 			} );
 
-		this.$next = $( '<div>' )
-			.attr( 'alt', mw.message( 'multimediaviewer-next-image-alt-text' ).text() )
+		this.$next = $( '<button>' )
+			.prop( 'title', mw.message( 'multimediaviewer-next-image-alt-text' ).text() )
 			.addClass( 'mw-mmv-next-image disabled' )
 			.html( '&nbsp;' );
 
-		this.$prev = $( '<div>' )
-			.attr( 'alt', mw.message( 'multimediaviewer-prev-image-alt-text' ).text() )
+		this.$prev = $( '<button>' )
+			.prop( 'title', mw.message( 'multimediaviewer-prev-image-alt-text' ).text() )
 			.addClass( 'mw-mmv-prev-image disabled' )
 			.html( '&nbsp;' );
 
diff --git a/resources/mmv/ui/mmv.ui.canvasButtons.less b/resources/mmv/ui/mmv.ui.canvasButtons.less
index 524e2ee13..bd73beb41 100644
--- a/resources/mmv/ui/mmv.ui.canvasButtons.less
+++ b/resources/mmv/ui/mmv.ui.canvasButtons.less
@@ -10,6 +10,7 @@
 .mw-mmv-prev-image {
 	cursor: pointer;
 	position: fixed;
+	background-color: transparent;
 	background-repeat: no-repeat;
 	.opacity( 0.8 );
 	border: 0;
diff --git a/tests/browser/features/support/pages/e2e_test_page.rb b/tests/browser/features/support/pages/e2e_test_page.rb
index 6344a118e..c0f040779 100644
--- a/tests/browser/features/support/pages/e2e_test_page.rb
+++ b/tests/browser/features/support/pages/e2e_test_page.rb
@@ -36,13 +36,13 @@ class E2ETestPage < CommonsPage
   a(:mmv_details_page_link, class: 'mw-mmv-description-page-button')
 
   # Controls
-  div(:mmv_next_button, class: 'mw-mmv-next-image')
-  div(:mmv_previous_button, class: 'mw-mmv-prev-image')
-  div(:mmv_close_button, class: 'mw-mmv-close')
+  button(:mmv_next_button, class: 'mw-mmv-next-image')
+  button(:mmv_previous_button, class: 'mw-mmv-prev-image')
+  button(:mmv_close_button, class: 'mw-mmv-close')
   div(:mmv_image_loaded_cucumber, class: 'mw-mmv-image-loaded-cucumber')
 
   # Download
-  div(:mmv_download_icon, class: 'mw-mmv-download-button')
+  button(:mmv_download_icon, class: 'mw-mmv-download-button')
   div(:mmv_download_menu, class: 'mw-mmv-download-dialog')
   span(:mmv_download_size_label, class: 'mw-mmv-download-image-size')
   span(:mmv_download_down_arrow_icon, class: 'mw-mmv-download-select-menu')
@@ -61,7 +61,7 @@ class E2ETestPage < CommonsPage
   end
 
   # Options
-  div(:mmv_options_icon, class: 'mw-mmv-options-button')
+  button(:mmv_options_icon, class: 'mw-mmv-options-button')
   div(:mmv_options_menu_disable, class: 'mw-mmv-options-disable')
   div(:mmv_options_menu_enable, class: 'mw-mmv-options-enable')
   button(:mmv_options_enable_button) do |page|
diff --git a/tests/qunit/mmv/mmv.lightboxinterface.test.js b/tests/qunit/mmv/mmv.lightboxinterface.test.js
index baf478487..29225812f 100644
--- a/tests/qunit/mmv/mmv.lightboxinterface.test.js
+++ b/tests/qunit/mmv/mmv.lightboxinterface.test.js
@@ -98,7 +98,7 @@
 		$.support.fullscreen = true;
 		lightbox.setupCanvasButtons();
 
-		assert.strictEqual( lightbox.$fullscreenButton.css( 'display' ), 'block',
+		assert.strictEqual( lightbox.$fullscreenButton.css( 'display' ), 'inline-block',
 			'Fullscreen button is visible when fullscreen mode is available' );
 
 		// Entering fullscreen