Bug 1482645 - Part 2 - Don't use a broadcaster element to store sync state. r=markh
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Sat, 11 Aug 2018 16:46:30 +0100
changeset 432386 8709ebdb4227339334ff169930a6308388f760bd
parent 432385 5ed210553fd0f580bd9369201596acda13d62690
child 432387 b43dff21dd79154d1b06b2e0318bfc9f79d41f28
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 2 - Don't use a broadcaster element to store sync state. r=markh Differential Revision: https://phabricator.services.mozilla.com/D3147
browser/base/content/browser-sets.inc
browser/base/content/browser-sync.js
browser/components/customizableui/content/panelUI.inc.xul
--- a/browser/base/content/browser-sets.inc
+++ b/browser/base/content/browser-sets.inc
@@ -121,21 +121,16 @@
 
 #include ../../components/places/content/placesCommands.inc.xul
 
   <broadcasterset id="mainBroadcasterSet">
     <broadcaster id="isImage"/>
     <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();"/>
     <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
@@ -116,24 +116,27 @@ var gSync = {
 
     this._definePrefGetters();
 
     if (!this.SYNC_ENABLED) {
       this.onSyncDisabled();
       return;
     }
 
-    // initial label for the sync buttons.
-    let statusBroadcaster = document.getElementById("sync-status");
-    if (!statusBroadcaster) {
+    // Label for the sync buttons, also set on the icon for accessibility.
+    let syncIcon = document.getElementById("appMenu-fxa-icon");
+    if (!syncIcon) {
       // 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"));
+    let syncNow = document.getElementById("PanelUI-remotetabs-syncnow");
+    let label = this.syncStrings.GetStringFromName("syncnow.label");
+    syncIcon.setAttribute("label", label);
+    syncNow.setAttribute("label", label);
     // 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).
     document.getElementById("sync-setup").hidden = false;
 
     for (let topic of this._obs) {
       Services.obs.addObserver(this, topic, true);
     }
 
@@ -254,18 +257,18 @@ var gSync = {
                                       "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";
+    let syncNow = document.getElementById("PanelUI-remotetabs-syncnow");
+    const syncingUI = syncNow.getAttribute("syncstatus") == "active";
     if (state.syncing != syncingUI) { // Do we need to update the UI?
       state.syncing ? this.onActivityStart() : this.onActivityStop();
     }
   },
 
   onMenuPanelCommand() {
     switch (this.appMenuContainer.getAttribute("fxastatus")) {
     case "signedin":
@@ -557,29 +560,39 @@ var gSync = {
                                            "disabled", !enabled || null);
   },
 
   // Functions called by observers
   onActivityStart() {
     clearTimeout(this._syncAnimationTimer);
     this._syncStartTime = Date.now();
 
-    let broadcaster = document.getElementById("sync-status");
-    broadcaster.setAttribute("syncstatus", "active");
-    broadcaster.setAttribute("label", this.syncStrings.GetStringFromName("syncingtabs.label"));
-    broadcaster.setAttribute("disabled", "true");
+    let label = this.syncStrings.GetStringFromName("syncingtabs.label");
+    let syncIcon = document.getElementById("appMenu-fxa-icon");
+    let syncNow = document.getElementById("PanelUI-remotetabs-syncnow");
+    syncIcon.setAttribute("syncstatus", "active");
+    syncIcon.setAttribute("label", label);
+    syncIcon.setAttribute("disabled", "true");
+    syncNow.setAttribute("syncstatus", "active");
+    syncNow.setAttribute("label", label);
+    syncNow.setAttribute("disabled", "true");
   },
 
   _onActivityStop() {
     if (!gBrowser)
       return;
-    let broadcaster = document.getElementById("sync-status");
-    broadcaster.removeAttribute("syncstatus");
-    broadcaster.removeAttribute("disabled");
-    broadcaster.setAttribute("label", this.syncStrings.GetStringFromName("syncnow.label"));
+    let label = this.syncStrings.GetStringFromName("syncnow.label");
+    let syncIcon = document.getElementById("appMenu-fxa-icon");
+    let syncNow = document.getElementById("PanelUI-remotetabs-syncnow");
+    syncIcon.removeAttribute("syncstatus");
+    syncIcon.removeAttribute("disabled");
+    syncIcon.setAttribute("label", label);
+    syncNow.removeAttribute("syncstatus");
+    syncNow.removeAttribute("disabled");
+    syncNow.setAttribute("label", label);
     Services.obs.notifyObservers(null, "test:browser-sync:activity-stop");
   },
 
   onActivityStop() {
     let now = Date.now();
     let syncDuration = now - this._syncStartTime;
 
     if (syncDuration < MIN_STATUS_ANIMATION_DURATION) {
@@ -634,18 +647,17 @@ var gSync = {
     }
   },
 
   refreshSyncButtonsTooltip() {
     const state = UIState.get();
     this.updateSyncButtonsTooltip(state);
   },
 
-  /* Update the tooltip for the sync-status broadcaster (which will update the
-     Sync Toolbar button and the Sync spinner in the FxA hamburger area.)
+  /* Update the tooltip for the sync icon in the main menu and in Synced Tabs.
      If Sync is configured, the tooltip is when the last sync occurred,
      otherwise the tooltip reflects the fact that Sync needs to be
      (re-)configured.
   */
   updateSyncButtonsTooltip(state) {
     const status = state.status;
 
     // This is a little messy as the Sync buttons are 1/2 Sync related and
@@ -661,22 +673,25 @@ var gSync = {
     } else if (status == UIState.STATUS_LOGIN_FAILED) {
       // "need to reconnect/re-enter your password"
       tooltiptext = this.fxaStrings.formatStringFromName("reconnectDescription", [state.email], 1);
     } else {
       // Sync appears configured - format the "last synced at" time.
       tooltiptext = this.formatLastSyncDate(state.lastSync);
     }
 
-    let broadcaster = document.getElementById("sync-status");
-    if (broadcaster) {
+    let syncIcon = document.getElementById("appMenu-fxa-icon");
+    if (syncIcon) {
+      let syncNow = document.getElementById("PanelUI-remotetabs-syncnow");
       if (tooltiptext) {
-        broadcaster.setAttribute("tooltiptext", tooltiptext);
+        syncIcon.setAttribute("tooltiptext", tooltiptext);
+        syncNow.setAttribute("tooltiptext", tooltiptext);
       } else {
-        broadcaster.removeAttribute("tooltiptext");
+        syncIcon.removeAttribute("tooltiptext");
+        syncNow.removeAttribute("tooltiptext");
       }
     }
   },
 
   get relativeTimeFormat() {
     delete this.relativeTimeFormat;
     return this.relativeTimeFormat = new Services.intl.RelativeTimeFormat(undefined, {style: "short"});
   },
--- a/browser/components/customizableui/content/panelUI.inc.xul
+++ b/browser/components/customizableui/content/panelUI.inc.xul
@@ -196,22 +196,19 @@
             <toolbarbutton id="appMenu-fxa-label"
                            class="subviewbutton subviewbutton-iconic"
                            label="&fxaSignIn.label;"
                            fxabrandname="&syncBrand.fxAccount.label;"/>
           </hbox>
           <toolbarseparator orient="vertical"/>
           <toolbarbutton id="appMenu-fxa-icon"
                          class="subviewbutton subviewbutton-iconic"
+                         onmouseover="gSync.refreshSyncButtonsTooltip();"
                          oncommand="gSync.doSync();"
-                         closemenu="none">
-            <observes element="sync-status" attribute="syncstatus"/>
-            <observes element="sync-status" attribute="tooltiptext"/>
-            <observes element="sync-status" attribute="onmouseover"/>
-          </toolbarbutton>
+                         closemenu="none"/>
         </toolbaritem>
         <toolbarseparator class="sync-ui-item"/>
         <toolbaritem closemenu="none">
           <toolbarbutton id="appMenu-tp-label"
                          tooltiptext="&trackingProtection.tooltip;"
                          class="subviewbutton subviewbutton-iconic"
                          oncommand="ContentBlocking.openPreferences('appMenu-trackingprotection'); PanelUI.hide();"
                          label="&trackingProtection.title;"/>
@@ -425,17 +422,17 @@
                            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;"
                            oncommand="gSync.openDevicesManagementPage('syncedtabs-menupanel');"/>
             <toolbarbutton id="PanelUI-remotetabs-syncnow"
-                           observes="sync-status"
+                           onmouseover="gSync.refreshSyncButtonsTooltip();"
                            class="subviewbutton subviewbutton-iconic"
                            oncommand="gSync.doSync();"
                            closemenu="none"/>
             <menuseparator id="PanelUI-remotetabs-separator"/>
           </vbox>
           <deck id="PanelUI-remotetabs-deck">
             <!-- Sync is ready to Sync and the "tabs" engine is enabled -->
             <vbox id="PanelUI-remotetabs-tabspane">