Bug 1378313 - Fix up bogus code showing the reload button as enabled when it's disabled. r=johannh draft
authorDão Gottwald <dao@mozilla.com>
Thu, 06 Jul 2017 12:32:29 +0200
changeset 604738 c08ddb408c2316bf957cffeed7ff12ffbb02bd0c
parent 604481 af0466865a212c84fbbab343c9cbb984c6132920
child 636281 46498733f4bea6e1a033c02688dd386ca0f69ce0
push id67176
push userdgottwald@mozilla.com
push dateThu, 06 Jul 2017 10:33:08 +0000
reviewersjohannh
bugs1378313, 1376893
milestone56.0a1
Bug 1378313 - Fix up bogus code showing the reload button as enabled when it's disabled. r=johannh Bug 1376893's approach is wrong in various ways: - It shows the reload button as enabled for about:blank - The disabled state styling is implemented in browser/themes/shared/toolbarbuttons.inc.css, and could be implemented differently. browser/base/content/browser.css should not depend on theme specifics. - :not(:-moz-window-inactive) only begins to make sense on Mac, and obviously prevents the fix from taking effect in inactive windows MozReview-Commit-ID: Dfh6VbirwPe
browser/base/content/browser.css
browser/base/content/browser.js
browser/base/content/test/permissions/browser_temporary_permissions_navigation.js
toolkit/components/passwordmgr/test/browser/browser_formless_submit_chrome.js
--- a/browser/base/content/browser.css
+++ b/browser/base/content/browser.css
@@ -469,22 +469,20 @@ toolbar:not(#TabsToolbar) > #personal-bo
   display: -moz-box;
 }
 
 #reload-button:not([displaystop]) + #stop-button,
 #reload-button[displaystop] {
   visibility: collapse;
 }
 
-/* The reload-button is only disabled temporarily when it becomes visible
-   to prevent users from accidentally clicking it. We don't however need
-   to show this disabled state, as the flicker that it generates is short
-   enough to be visible but not long enough to explain anything to users. */
-#reload-button[disabled]:not(:-moz-window-inactive) > .toolbarbutton-icon {
-  opacity: 1 !important;
+/* Temporarily disable the reload button to prevent the user from
+   accidentally reloading the page when intending to click the stop button. */
+#reload-button[temporarily-disabled] {
+  pointer-events: none;
 }
 
 #PanelUI-feeds > .feed-toolbarbutton:-moz-locale-dir(rtl) {
   direction: rtl;
 }
 
 #panelMenu_bookmarksMenu > .bookmark-item {
   max-width: none;
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -4963,21 +4963,20 @@ var CombinedStopReload = {
       return;
     }
 
     if (this._timer)
       return;
 
     // Temporarily disable the reload button to prevent the user from
     // accidentally reloading the page when intending to click the stop button
-    this.reload.disabled = true;
+    this.reload.setAttribute("temporarily-disabled", "true");
     this._timer = setTimeout(function(self) {
       self._timer = 0;
-      self.reload.disabled = XULBrowserWindow.reloadCommand
-                                             .getAttribute("disabled") == "true";
+      self.reload.removeAttribute("temporarily-disabled");
     }, 650, this);
   },
 
   _cancelTransition() {
     if (this._timer) {
       clearTimeout(this._timer);
       this._timer = 0;
     }
--- a/browser/base/content/test/permissions/browser_temporary_permissions_navigation.js
+++ b/browser/base/content/test/permissions/browser_temporary_permissions_navigation.js
@@ -19,19 +19,18 @@ add_task(async function testTempPermissi
       state: SitePermissions.BLOCK,
       scope: SitePermissions.SCOPE_TEMPORARY,
     });
 
     // Reload through the page (should not remove the temp permission).
     await ContentTask.spawn(browser, {}, () => content.document.location.reload());
 
     await reloaded;
-    await BrowserTestUtils.waitForCondition(() => {
-      return reloadButton.disabled == false;
-    });
+    await BrowserTestUtils.waitForCondition(() =>
+      !reloadButton.disabled && !reloadButton.hasAttribute("temporarily-disabled"));
 
     Assert.deepEqual(SitePermissions.get(uri, id, browser), {
       state: SitePermissions.BLOCK,
       scope: SitePermissions.SCOPE_TEMPORARY,
     });
 
     reloaded = BrowserTestUtils.browserLoaded(browser, false, uri.spec);
 
--- a/toolkit/components/passwordmgr/test/browser/browser_formless_submit_chrome.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_formless_submit_chrome.js
@@ -100,19 +100,18 @@ add_task(async function test_backButton_
 
 add_task(async function test_reloadButton() {
   await withTestPage(async function(aBrowser) {
     let reloadButton = document.getElementById("reload-button");
     let loadPromise = BrowserTestUtils.browserLoaded(aBrowser, false,
                                                      "https://example.com" + DIRECTORY_PATH +
                                                      "formless_basic.html");
 
-    await BrowserTestUtils.waitForCondition(() => {
-      return reloadButton.disabled == false;
-    });
+    await BrowserTestUtils.waitForCondition(() =>
+      !reloadButton.disabled && !reloadButton.hasAttribute("temporarily-disabled"));
     EventUtils.synthesizeMouseAtCenter(reloadButton, {});
     await loadPromise;
   });
 });
 
 add_task(async function test_back_keyboard_shortcut() {
   if (Services.prefs.getIntPref("browser.backspace_action") != 0) {
     ok(true, "Skipped testing backspace to go back since it's disabled");