Bug 1482645 - Part 1 - Don't use broadcaster elements to show or hide sync interface elements. r=markh
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Mon, 20 Aug 2018 07:38:40 +0100
changeset 432385 5ed210553fd0f580bd9369201596acda13d62690
parent 432384 be8aca082faf9aef30236657c9c486750e831a21
child 432386 8709ebdb4227339334ff169930a6308388f760bd
push id106721
push userpaolo.mozmail@amadzone.org
push dateMon, 20 Aug 2018 15:49:07 +0000
treeherdermozilla-inbound@8709ebdb4227 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1482645
milestone63.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 1482645 - Part 1 - Don't use broadcaster elements to show or hide sync interface elements. r=markh Differential Revision: https://phabricator.services.mozilla.com/D3146
browser/base/content/browser-menubar.inc
browser/base/content/browser-sets.inc
browser/base/content/browser-sync.js
browser/components/customizableui/content/panelUI.inc.xul
--- a/browser/base/content/browser-menubar.inc
+++ b/browser/base/content/browser-menubar.inc
@@ -484,35 +484,35 @@
                         key="key_openAddons"
                         command="Tools:Addons"/>
 
               <!-- only one of sync-setup, sync-unverifieditem, sync-syncnowitem or sync-reauthitem will be showing at once -->
               <menuitem id="sync-setup"
                         class="sync-ui-item"
                         label="&syncSignIn.label;"
                         accesskey="&syncSignIn.accesskey;"
-                        observes="sync-setup-state"
+                        hidden="true"
                         oncommand="gSync.openPrefs('menubar')"/>
               <menuitem id="sync-unverifieditem"
                         class="sync-ui-item"
                         label="&syncSignIn.label;"
                         accesskey="&syncSignIn.accesskey;"
-                        observes="sync-unverified-state"
+                        hidden="true"
                         oncommand="gSync.openPrefs('menubar')"/>
               <menuitem id="sync-syncnowitem"
                         class="sync-ui-item"
                         label="&syncSyncNowItem.label;"
                         accesskey="&syncSyncNowItem.accesskey;"
-                        observes="sync-syncnow-state"
+                        hidden="true"
                         oncommand="gSync.doSync(event);"/>
               <menuitem id="sync-reauthitem"
                         class="sync-ui-item"
                         label="&syncReAuthItem.label;"
                         accesskey="&syncReAuthItem.accesskey;"
-                        observes="sync-reauth-state"
+                        hidden="true"
                         oncommand="gSync.openSignInAgainPage('menubar');"/>
               <menuseparator id="devToolsSeparator"/>
               <menu id="webDeveloperMenu"
                     label="&webDeveloperMenu.label;"
                     accesskey="&webDeveloperMenu.accesskey;">
                 <menupopup id="menuWebDeveloperPopup">
                   <menuitem id="menu_pageSource"
                             label="&pageSourceCmd.label;"
--- a/browser/base/content/browser-sets.inc
+++ b/browser/base/content/browser-sets.inc
@@ -126,22 +126,16 @@
     <broadcaster id="canViewSource"/>
     <broadcaster id="isFrameImage"/>
 
     <!-- Sync broadcasters -->
     <!-- A broadcaster of a number of attributes suitable for "sync now" UI -
         A 'syncstatus' attribute is set while actively syncing, and the label
         attribute which changes from "sync now" to "syncing" etc. -->
     <broadcaster id="sync-status" onmouseover="gSync.refreshSyncButtonsTooltip();"/>
-    <!-- broadcasters of the "hidden" attribute to reflect setup state for
-         menus -->
-    <broadcaster id="sync-setup-state" hidden="true"/>
-    <broadcaster id="sync-unverified-state" hidden="true"/>
-    <broadcaster id="sync-syncnow-state" hidden="true"/>
-    <broadcaster id="sync-reauth-state" hidden="true"/>
     <broadcaster id="workOfflineMenuitemState"/>
     <broadcaster id="devtoolsMenuBroadcaster_RecordExecution"
                  label="&devtoolsRecordExecution.label;"
                  command="Tools:RecordExecution"/>
     <broadcaster id="devtoolsMenuBroadcaster_SaveRecording"
                  label="&devtoolsSaveRecording.label;"
                  command="Tools:SaveRecording"/>
     <broadcaster id="devtoolsMenuBroadcaster_ReplayExecution"
--- a/browser/base/content/browser-sync.js
+++ b/browser/base/content/browser-sync.js
@@ -124,20 +124,19 @@ var gSync = {
     // initial label for the sync buttons.
     let statusBroadcaster = document.getElementById("sync-status");
     if (!statusBroadcaster) {
       // We are in a window without our elements - just abort now, without
       // setting this._initialized, so we don't attempt to remove observers.
       return;
     }
     statusBroadcaster.setAttribute("label", this.syncStrings.GetStringFromName("syncnow.label"));
-    // We start with every broadcasters hidden, so that we don't need to init
+    // We start with every menuitem hidden, so that we don't need to init
     // the sync UI on windows like pageInfo.xul (see bug 1384856).
-    let setupBroadcaster = document.getElementById("sync-setup-state");
-    setupBroadcaster.hidden = false;
+    document.getElementById("sync-setup").hidden = false;
 
     for (let topic of this._obs) {
       Services.obs.addObserver(this, topic, true);
     }
 
     this._generateNodeGetters();
 
     this._maybeUpdateUIState();
@@ -180,17 +179,17 @@ var gSync = {
         }
         this.onClientsSynced();
         break;
     }
   },
 
   updateAllUI(state) {
     this.updatePanelPopup(state);
-    this.updateStateBroadcasters(state);
+    this.updateState(state);
     this.updateSyncButtonsTooltip(state);
     this.updateSyncStatus(state);
   },
 
   updatePanelPopup(state) {
     let defaultLabel = this.appMenuStatus.getAttribute("defaultlabel");
     // The localization string is for the signed in text, but it's the default text as well
     let defaultTooltiptext = this.appMenuStatus.getAttribute("signedinTooltiptext");
@@ -238,34 +237,29 @@ var gSync = {
         if (this.appMenuAvatar.style.listStyleImage === bgImage) {
           this.appMenuAvatar.style.removeProperty("list-style-image");
         }
       };
       img.src = state.avatarURL;
     }
   },
 
-  updateStateBroadcasters(state) {
-    const status = state.status;
-
-    // Start off with a clean slate
-    document.getElementById("sync-reauth-state").hidden = true;
-    document.getElementById("sync-setup-state").hidden = true;
-    document.getElementById("sync-syncnow-state").hidden = true;
-    document.getElementById("sync-unverified-state").hidden = true;
-
-    if (status == UIState.STATUS_LOGIN_FAILED) {
-      // unhiding this element makes the menubar show the login failure state.
-      document.getElementById("sync-reauth-state").hidden = false;
-    } else if (status == UIState.STATUS_NOT_CONFIGURED) {
-      document.getElementById("sync-setup-state").hidden = false;
-    } else if (status == UIState.STATUS_NOT_VERIFIED) {
-      document.getElementById("sync-unverified-state").hidden = false;
-    } else {
-      document.getElementById("sync-syncnow-state").hidden = false;
+  updateState(state) {
+    for (let [status, menuId, boxId] of [
+      [UIState.STATUS_NOT_CONFIGURED, "sync-setup",
+                                      "PanelUI-remotetabs-setupsync"],
+      [UIState.STATUS_LOGIN_FAILED,   "sync-reauthitem",
+                                      "PanelUI-remotetabs-reauthsync"],
+      [UIState.STATUS_NOT_VERIFIED,   "sync-unverifieditem",
+                                      "PanelUI-remotetabs-unverified"],
+      [UIState.STATUS_SIGNED_IN,      "sync-syncnowitem",
+                                      "PanelUI-remotetabs-main"],
+    ]) {
+      document.getElementById(menuId).hidden =
+        document.getElementById(boxId).hidden = (status != state.status);
     }
   },
 
   updateSyncStatus(state) {
     const broadcaster = document.getElementById("sync-status");
     const syncingUI = broadcaster.getAttribute("syncstatus") == "active";
     if (state.syncing != syncingUI) { // Do we need to update the UI?
       state.syncing ? this.onActivityStart() : this.onActivityStop();
@@ -691,22 +685,22 @@ var gSync = {
     if (!date) { // Date can be null before the first sync!
       return null;
     }
     const relativeDateStr = this.relativeTimeFormat.formatBestUnit(date);
     return this.syncStrings.formatStringFromName("lastSync2.label", [relativeDateStr], 1);
   },
 
   onClientsSynced() {
-    let broadcaster = document.getElementById("sync-syncnow-state");
-    if (broadcaster) {
+    let element = document.getElementById("PanelUI-remotetabs-main");
+    if (element) {
       if (Weave.Service.clientsEngine.stats.numClients > 1) {
-        broadcaster.setAttribute("devices-status", "multi");
+        element.setAttribute("devices-status", "multi");
       } else {
-        broadcaster.setAttribute("devices-status", "single");
+        element.setAttribute("devices-status", "single");
       }
     }
   },
 
   onSyncDisabled() {
     const toHide = [...document.querySelectorAll(".sync-ui-item")];
     for (const item of toHide) {
       item.hidden = true;
--- a/browser/components/customizableui/content/panelUI.inc.xul
+++ b/browser/components/customizableui/content/panelUI.inc.xul
@@ -414,17 +414,17 @@
     <panelview id="appMenu-library-recentlyClosedTabs"/>
     <panelview id="appMenu-library-recentlyClosedWindows"/>
 
     <panelview id="PanelUI-remotetabs" flex="1" class="PanelUI-subView"
                descriptionheightworkaround="true">
       <vbox class="panel-subview-body">
         <!-- this widget has 3 boxes in the body, but only 1 is ever visible -->
         <!-- When Sync is ready to sync -->
-        <vbox id="PanelUI-remotetabs-main" observes="sync-syncnow-state">
+        <vbox id="PanelUI-remotetabs-main" hidden="true">
           <vbox id="PanelUI-remotetabs-buttons">
             <toolbarbutton id="PanelUI-remotetabs-view-sidebar"
                            class="subviewbutton subviewbutton-iconic"
                            label="&appMenuRemoteTabs.sidebar.label;"
                            oncommand="SidebarUI.toggle('viewTabsSidebar');"/>
             <toolbarbutton id="PanelUI-remotetabs-view-managedevices"
                            class="subviewbutton subviewbutton-iconic"
                            label="&appMenuRemoteTabs.managedevices.label;"
@@ -482,42 +482,41 @@
         </vbox>
         <!-- a box to ensure contained boxes are centered horizonally -->
         <hbox pack="center" flex="1">
           <!-- When Sync is not configured -->
           <vbox id="PanelUI-remotetabs-setupsync"
                 flex="1"
                 align="center"
                 class="PanelUI-remotetabs-instruction-box"
-                observes="sync-setup-state">
+                hidden="true">
             <image class="fxaSyncIllustration"/>
             <label class="PanelUI-remotetabs-instruction-label">&appMenuRemoteTabs.notsignedin.label;</label>
             <toolbarbutton class="PanelUI-remotetabs-button"
                            label="&appMenuRemoteTabs.signin.label;"
                            oncommand="gSync.openPrefs('synced-tabs');"/>
           </vbox>
-          <!-- When Sync needs re-authentication. This uses the exact same messaging
-               as "Sync is not configured" but remains a separate box so we get
-               the goodness of observing broadcasters to manage the hidden states -->
+          <!-- When Sync needs re-authentication -->
           <vbox id="PanelUI-remotetabs-reauthsync"
                 flex="1"
                 align="center"
                 class="PanelUI-remotetabs-instruction-box"
-                observes="sync-reauth-state">
+                hidden="true">
             <image class="fxaSyncIllustrationIssue"/>
             <label class="PanelUI-remotetabs-instruction-label">&appMenuRemoteTabs.notsignedin.label;</label>
             <toolbarbutton class="PanelUI-remotetabs-button"
                            label="&appMenuRemoteTabs.signin.label;"
                            oncommand="gSync.openPrefs('synced-tabs');"/>
           </vbox>
+          <!-- When Sync needs verification -->
           <vbox id="PanelUI-remotetabs-unverified"
                 flex="1"
                 align="center"
                 class="PanelUI-remotetabs-instruction-box"
-                observes="sync-unverified-state">
+                hidden="true">
             <image class="fxaSyncIllustrationIssue"/>
             <label class="PanelUI-remotetabs-instruction-label">&appMenuRemoteTabs.unverified.label;</label>
             <toolbarbutton class="PanelUI-remotetabs-button"
                            label="&appMenuRemoteTabs.opensyncprefs.label;"
                            oncommand="gSync.openPrefs('synced-tabs');"/>
           </vbox>
         </hbox>
       </vbox>