Bug 1494014 - fix icon on the lightweight theme button to be correct when changing it from the menu, r=jaws
☠☠ backed out by df7e7d55004d ☠ ☠
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 21 Nov 2018 15:08:03 +0000
changeset 503905 6ce32daf80793ae70f50cfdd421aad837e155435
parent 503904 f340a2489f7366a1383ac842ac3191a19e26b500
child 503906 adab030a385aa769613b3825073b3ba96af65a68
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,25 @@ 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");
+  is(themesButtonIcon.style.backgroundImage, "",
+     `Button should show default theme thumbnail (empty string) - was: "${themesButtonIcon.style.backgroundImage}"`);
   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 +52,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) {