Bug 1494014 - fix icon on the lightweight theme button to be correct when changing it from the menu, r=jaws
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 21 Nov 2018 17:23:05 +0000
changeset 503940 929c91b8afc67ab6b89df4f242dcfbb5fbf21019
parent 503939 023c546c01cd2d3c4d382ab5e9245a48f138e934
child 503941 3d997ec4174de4296a6bd30641d42309999703b0
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1494014
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1494014 - fix icon on the lightweight theme button to be correct when changing it from the menu, r=jaws Differential Revision: https://phabricator.services.mozilla.com/D12546
browser/components/customizableui/CustomizeMode.jsm
browser/components/customizableui/test/browser_1007336_lwthemes_in_customize_mode.js
--- a/browser/components/customizableui/CustomizeMode.jsm
+++ b/browser/components/customizableui/CustomizeMode.jsm
@@ -162,18 +162,18 @@ CustomizeMode.prototype = {
       this.enter();
     }
   },
 
   _updateLWThemeButtonIcon() {
     let lwthemeButton = this.$("customization-lwtheme-button");
     let lwthemeIcon = this.document.getAnonymousElementByAttribute(lwthemeButton,
                         "class", "button-icon");
-    lwthemeIcon.style.backgroundImage = LightweightThemeManager.currentTheme ?
-      "url(" + LightweightThemeManager.currentTheme.iconURL + ")" : "";
+    let theme = LightweightThemeManager.currentTheme;
+    lwthemeIcon.style.backgroundImage = theme ? "url(" + theme.iconURL + ")" : "";
   },
 
   setTab(aTab) {
     if (gTab == aTab) {
       return;
     }
 
     if (gTab) {
@@ -339,16 +339,17 @@ CustomizeMode.prototype = {
         // Force layout reflow to ensure the animation runs,
         // and make it async so it doesn't affect the timing.
         this.visiblePalette.clientTop;
         this.visiblePalette.setAttribute("showing", "true");
       }, 0);
       this._updateEmptyPaletteNotice();
 
       this._updateLWThemeButtonIcon();
+      Services.obs.addObserver(this, "lightweight-theme-changed");
 
       this._setupDownloadAutoHideToggle();
 
       this._handler.isEnteringCustomizeMode = false;
 
       CustomizableUI.dispatchToolboxEvent("customizationready", {}, window);
 
       if (!this._wantToBeInCustomizeMode) {
@@ -381,16 +382,17 @@ CustomizeMode.prototype = {
                 "We'll exit after resetting has finished.");
       return;
     }
 
     this._handler.isExitingCustomizeMode = true;
 
     this._teardownDownloadAutoHideToggle();
 
+    Services.obs.removeObserver(this, "lightweight-theme-changed");
     CustomizableUI.removeListener(this);
 
     this.document.removeEventListener("keypress", this);
 
     let window = this.window;
     let document = this.document;
 
     this.togglePong(false);
@@ -1063,18 +1065,16 @@ CustomizeMode.prototype = {
     let btn = this.$("customization-reset-button");
     btn.disabled = true;
     return (async () => {
       await this.depopulatePalette();
       await this._unwrapToolbarItems();
 
       CustomizableUI.reset();
 
-      this._updateLWThemeButtonIcon();
-
       await this._wrapToolbarItems();
       this.populatePalette();
 
       this._updateResetButton();
       this._updateUndoResetButton();
       this._updateEmptyPaletteNotice();
       this._moveDownloadsButtonToNavBar = false;
       this.resetting = false;
@@ -1088,18 +1088,16 @@ CustomizeMode.prototype = {
     this.resetting = true;
 
     return (async () => {
       await this.depopulatePalette();
       await this._unwrapToolbarItems();
 
       CustomizableUI.undoReset();
 
-      this._updateLWThemeButtonIcon();
-
       await this._wrapToolbarItems();
       this.populatePalette();
 
       this._updateResetButton();
       this._updateUndoResetButton();
       this._updateEmptyPaletteNotice();
       this._moveDownloadsButtonToNavBar = false;
       this.resetting = false;
@@ -1323,18 +1321,19 @@ CustomizeMode.prototype = {
         aPreviewThemeEvent.target.theme : null);
     }
 
     function resetPreview() {
       LightweightThemeManager.resetPreview();
     }
 
     let onThemeSelected = panel => {
-      this._updateLWThemeButtonIcon();
-      this._onUIChange();
+      // This causes us to call _onUIChange when the LWT actually changes,
+      // so the restore defaults / undo reset button is updated correctly.
+      this._nextThemeChangeUserTriggered = true;
       panel.hidePopup();
     };
 
     let doc = this.window.document;
 
     function buildToolbarButton(aTheme) {
       let tbb = doc.createXULElement("toolbarbutton");
       tbb.theme = aTheme;
@@ -1563,16 +1562,23 @@ CustomizeMode.prototype = {
       case "nsPref:changed":
         this._updateResetButton();
         this._updateUndoResetButton();
         if (this._canDrawInTitlebar()) {
           this._updateTitlebarCheckbox();
           this._updateDragSpaceCheckbox();
         }
         break;
+      case "lightweight-theme-changed":
+        this._updateLWThemeButtonIcon();
+        if (this._nextThemeChangeUserTriggered) {
+          this._onUIChange();
+        }
+        this._nextThemeChangeUserTriggered = false;
+        break;
     }
   },
 
   _canDrawInTitlebar() {
     return this.window.TabsInTitlebar.systemSupported;
   },
 
   _updateTitlebarCheckbox() {
--- a/browser/components/customizableui/test/browser_1007336_lwthemes_in_customize_mode.js
+++ b/browser/components/customizableui/test/browser_1007336_lwthemes_in_customize_mode.js
@@ -9,18 +9,35 @@ const LIGHT_THEME_ID = "firefox-compact-
 const DARK_THEME_ID = "firefox-compact-dark@mozilla.org";
 const {LightweightThemeManager} = ChromeUtils.import("resource://gre/modules/LightweightThemeManager.jsm", {});
 
 add_task(async function() {
   Services.prefs.clearUserPref("lightweightThemes.usedThemes");
   Services.prefs.clearUserPref("lightweightThemes.recommendedThemes");
 
   await startCustomizing();
+  // Check restore defaults button is disabled.
+  ok(document.getElementById("customization-reset-button").disabled,
+     "Reset button should start out disabled");
 
   let themesButton = document.getElementById("customization-lwtheme-button");
+  let themesButtonIcon = document.getAnonymousElementByAttribute(themesButton,
+      "class", "button-icon");
+  let iconURL = themesButtonIcon.style.backgroundImage;
+  // If we've run other tests before, we might have set the image to the
+  // default theme's icon explicitly, otherwise it might be empty, in which
+  // case the icon is determined by CSS (which will be the default
+  // theme's icon).
+  if (iconURL) {
+    ok((/default/i).test(themesButtonIcon.style.backgroundImage),
+       `Button should show default theme thumbnail - was: "${iconURL}"`);
+  } else {
+    is(iconURL, "",
+       `Button should show default theme thumbnail (empty string) - was: "${iconURL}"`);
+  }
   let popup = document.getElementById("customization-lwtheme-menu");
 
   let popupShownPromise = popupShown(popup);
   EventUtils.synthesizeMouseAtCenter(themesButton, {});
   info("Clicked on themes button");
   await popupShownPromise;
 
   // close current tab and re-open Customize menu to confirm correct number of Themes
@@ -45,16 +62,22 @@ add_task(async function() {
   is(header.nextElementSibling.nextElementSibling.nextElementSibling.theme.id, DARK_THEME_ID,
      "The third theme should be the dark theme");
 
   let themeChangedPromise = promiseObserverNotified("lightweight-theme-changed");
   header.nextElementSibling.nextElementSibling.doCommand(); // Select light theme
   info("Clicked on light theme");
   await themeChangedPromise;
 
+  // Check restore defaults button is enabled.
+  ok(!document.getElementById("customization-reset-button").disabled,
+     "Reset button should not be disabled anymore");
+  ok((/light/i).test(themesButtonIcon.style.backgroundImage),
+     `Button should show light theme thumbnail - was: "${themesButtonIcon.style.backgroundImage}"`);
+
   popupShownPromise = popupShown(popup);
   EventUtils.synthesizeMouseAtCenter(themesButton, {});
   info("Clicked on themes button a third time");
   await popupShownPromise;
 
   let activeThemes = popup.querySelectorAll("toolbarbutton.customization-lwtheme-menu-theme[active]");
   is(activeThemes.length, 1, "Exactly 1 theme should be selected");
   if (activeThemes.length > 0) {