Bug 1244597 - Show notification on incoming tab. r=markh
☠☠ backed out by 98979093d45f ☠ ☠
authorEdouard Oger <eoger@fastmail.com>
Thu, 07 Jul 2016 13:33:29 -0700
changeset 305220 53236b4393f2da5da9925b511d6e702860f9319f
parent 305219 9125083b8dfeb57a6ac6b16e26a020987e3e7295
child 305221 3db372da6fe108ddb0727003447f22c2483fb86d
push id79518
push usercbook@mozilla.com
push dateSun, 17 Jul 2016 08:09:59 +0000
treeherdermozilla-inbound@711963e8daa3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1244597
milestone50.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 1244597 - Show notification on incoming tab. r=markh MozReview-Commit-ID: BW7irvjVGiv
browser/components/nsBrowserGlue.js
browser/locales/en-US/chrome/browser/accounts.properties
services/sync/modules/engines/clients.js
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -310,18 +310,18 @@ BrowserGlue.prototype = {
         this._setSyncAutoconnectDelay();
         break;
       case "fxaccounts:onverified":
         this._showSyncStartedDoorhanger();
         break;
       case "fxaccounts:device_disconnected":
         this._onDeviceDisconnected();
         break;
-      case "weave:engine:clients:display-uri":
-        this._onDisplaySyncURI(subject);
+      case "weave:engine:clients:display-uris":
+        this._onDisplaySyncURIs(subject);
         break;
       case "session-save":
         this._setPrefToSaveSession(true);
         subject.QueryInterface(Ci.nsISupportsPRBool);
         subject.data = true;
         break;
       case "places-init-complete":
         if (!this._migrationImportsDefaultBookmarks)
@@ -529,17 +529,17 @@ BrowserGlue.prototype = {
     os.addObserver(this, "quit-application-granted", false);
     if (OBSERVE_LASTWINDOW_CLOSE_TOPICS) {
       os.addObserver(this, "browser-lastwindow-close-requested", false);
       os.addObserver(this, "browser-lastwindow-close-granted", false);
     }
     os.addObserver(this, "weave:service:ready", false);
     os.addObserver(this, "fxaccounts:onverified", false);
     os.addObserver(this, "fxaccounts:device_disconnected", false);
-    os.addObserver(this, "weave:engine:clients:display-uri", false);
+    os.addObserver(this, "weave:engine:clients:display-uris", false);
     os.addObserver(this, "session-save", false);
     os.addObserver(this, "places-init-complete", false);
     this._isPlacesInitObserver = true;
     os.addObserver(this, "places-database-locked", false);
     this._isPlacesLockedObserver = true;
     os.addObserver(this, "distribution-customization-complete", false);
     os.addObserver(this, "places-shutdown", false);
     this._isPlacesShutdownObserver = true;
@@ -596,17 +596,17 @@ BrowserGlue.prototype = {
     os.removeObserver(this, "restart-in-safe-mode");
     if (OBSERVE_LASTWINDOW_CLOSE_TOPICS) {
       os.removeObserver(this, "browser-lastwindow-close-requested");
       os.removeObserver(this, "browser-lastwindow-close-granted");
     }
     os.removeObserver(this, "weave:service:ready");
     os.removeObserver(this, "fxaccounts:onverified");
     os.removeObserver(this, "fxaccounts:device_disconnected");
-    os.removeObserver(this, "weave:engine:clients:display-uri");
+    os.removeObserver(this, "weave:engine:clients:display-uris");
     os.removeObserver(this, "session-save");
     if (this._bookmarksBackupIdleTime) {
       this._idleService.removeIdleObserver(this, this._bookmarksBackupIdleTime);
       delete this._bookmarksBackupIdleTime;
     }
     if (this._isPlacesInitObserver)
       os.removeObserver(this, "places-init-complete");
     if (this._isPlacesLockedObserver)
@@ -2429,34 +2429,69 @@ BrowserGlue.prototype = {
       return;
     }
 
     let chromeWindow = RecentWindow.getMostRecentBrowserWindow();
     chromeWindow.openPreferences(...args);
   },
 
   /**
-   * Called as an observer when Sync's "display URI" notification is fired.
-   *
-   * We open the received URI in a background tab.
+   * Called as an observer when Sync's "display URIs" notification is fired.
    *
-   * Eventually, this will likely be replaced by a more robust tab syncing
-   * feature. This functionality is considered somewhat evil by UX because it
-   * opens a new tab automatically without any prompting. However, it is a
-   * lesser evil than sending a tab to a specific device (from e.g. Fennec)
-   * and having nothing happen on the receiving end.
+   * We open the received URIs in background tabs.
    */
-  _onDisplaySyncURI: function _onDisplaySyncURI(data) {
+  _onDisplaySyncURIs: function _onDisplaySyncURIs(data) {
     try {
-      let tabbrowser = RecentWindow.getMostRecentBrowserWindow({private: false}).gBrowser;
-
       // The payload is wrapped weirdly because of how Sync does notifications.
-      tabbrowser.addTab(data.wrappedJSObject.object.uri);
+      const URIs = data.wrappedJSObject.object;
+
+      const findWindow = () => RecentWindow.getMostRecentBrowserWindow({private: false});
+
+      // win can be null, but it's ok, we'll assign it later in openTab()
+      let win = findWindow();
+
+      const openTab = URI => {
+        let tab;
+        if (!win) {
+          Services.appShell.hiddenDOMWindow.open(URI.uri);
+          win = findWindow();
+          tab = win.gBrowser.tabs[0];
+        } else {
+          tab = win.gBrowser.addTab(URI.uri);
+        }
+        tab.setAttribute("attention", true);
+        return tab;
+      };
+
+      const firstTab = openTab(URIs[0]);
+      URIs.slice(1).forEach(URI => openTab(URI));
+
+      let title, body;
+      const deviceName = Weave.Service.clientsEngine.getClientName(URIs[0].clientId);
+      const bundle = Services.strings.createBundle("chrome://browser/locale/accounts.properties");
+      if (URIs.length == 1) {
+        title = bundle.GetStringFromName("tabArrivingNotification.title");
+        const pageTitle = URIs[0].title || firstTab.linkedBrowser.contentTitle
+                          || URIs[0].uri;
+        body = bundle.formatStringFromName("tabArrivingNotification.body", [pageTitle, deviceName], 2);
+      } else {
+        title = bundle.GetStringFromName("tabsArrivingNotification.title");
+        const tabArrivingBody = URIs.every(URI => URI.clientId == URIs[0].clientId) ?
+                                "tabsArrivingNotification.body" : "tabsArrivingNotificationMultiple.body";
+        body = bundle.formatStringFromName(tabArrivingBody, [URIs.length, deviceName], 2);
+      }
+
+      const clickCallback = (subject, topic, data) => {
+        if (topic == "alertclickcallback") {
+          win.gBrowser.selectedTab = firstTab;
+        }
+      }
+      AlertsService.showAlertNotification(null, title, body, true, null, clickCallback);
     } catch (ex) {
-      Cu.reportError("Error displaying tab received by Sync: " + ex);
+      Cu.reportError("Error displaying tab(s) received by Sync: " + ex);
     }
   },
 
   _onDeviceDisconnected() {
     let bundle = Services.strings.createBundle("chrome://browser/locale/accounts.properties");
     let title = bundle.GetStringFromName("deviceDisconnectedNotification.title");
     let body = bundle.GetStringFromName("deviceDisconnectedNotification.body");
 
--- a/browser/locales/en-US/chrome/browser/accounts.properties
+++ b/browser/locales/en-US/chrome/browser/accounts.properties
@@ -28,8 +28,23 @@ verificationNotSentBody = We are unable 
 # These strings are used in a notification shown after Sync is connected.
 syncStartNotification.title = Sync enabled
 syncStartNotification.body = Firefox will begin syncing momentarily.
 
 # LOCALIZATION NOTE (deviceDisconnectedNotification.title, deviceDisconnectedNotification.body)
 # These strings are used in a notification shown after Sync was disconnected remotely.
 deviceDisconnectedNotification.title = Sync disconnected
 deviceDisconnectedNotification.body = This computer has been successfully disconnected from Firefox Sync.
+
+# LOCALIZATION NOTE (tabArrivingNotification.title, tabArrivingNotification.body,
+# tabsArrivingNotification.title, tabsArrivingNotification.body)
+# These strings are used in a notification shown when we're opening tab(s) another device sent us to display.
+tabArrivingNotification.title = Tab received
+# LOCALIZATION NOTE (tabArrivingNotification.body) %1 is the title of the tab and %2 is the device name.
+tabArrivingNotification.body = "%1$S" has arrived from %2$S.
+
+tabsArrivingNotification.title = Multiple tabs received
+# LOCALIZATION NOTE (tabsArrivingNotification.body) %1 is the number of tabs received and %2 is the device name.
+tabsArrivingNotification.body = %1$S tabs have arrived from %2$S.
+# LOCALIZATION NOTE (tabsArrivingNotificationMultiple.body)
+# This string is used in a notification shown when we're opening tab(s) that several devices sent us to display.
+# %S is the number of tabs received
+tabsArrivingNotificationMultiple.body = %S tabs have arrived from your connected devices.
--- a/services/sync/modules/engines/clients.js
+++ b/services/sync/modules/engines/clients.js
@@ -374,16 +374,17 @@ ClientEngine.prototype = {
 
       // Immediately clear out the commands as we've got them locally.
       this.clearCommands();
 
       // Process each command in order.
       if (!commands) {
         return true;
       }
+      let URIsToDisplay = [];
       for (let key in commands) {
         let {command, args} = commands[key];
         this._log.debug("Processing command: " + command + "(" + args + ")");
 
         let engines = [args[0]];
         switch (command) {
           case "resetAll":
             engines = null;
@@ -396,23 +397,27 @@ ClientEngine.prototype = {
             // Fallthrough
           case "wipeEngine":
             this.service.wipeClient(engines);
             break;
           case "logout":
             this.service.logout();
             return false;
           case "displayURI":
-            this._handleDisplayURI.apply(this, args);
+            let [uri, clientId, title] = args;
+            URIsToDisplay.push({ uri, clientId, title });
             break;
           default:
             this._log.debug("Received an unknown command: " + command);
             break;
         }
       }
+      if (URIsToDisplay.length) {
+        this._handleDisplayURIs(URIsToDisplay);
+      }
 
       return true;
     })();
   },
 
   /**
    * Validates and sends a command to a client or all clients.
    *
@@ -472,42 +477,40 @@ ClientEngine.prototype = {
     this._log.info("Sending URI to client: " + uri + " -> " +
                    clientId + " (" + title + ")");
     this.sendCommand("displayURI", [uri, this.localID, title], clientId);
 
     this._tracker.score += SCORE_INCREMENT_XLARGE;
   },
 
   /**
-   * Handle a single received 'displayURI' command.
+   * Handle a bunch of received 'displayURI' commands.
    *
-   * Interested parties should observe the "weave:engine:clients:display-uri"
-   * topic. The callback will receive an object as the subject parameter with
-   * the following keys:
+   * Interested parties should observe the "weave:engine:clients:display-uris"
+   * topic. The callback will receive an array as the subject parameter
+   * containing objects with the following keys:
    *
    *   uri       URI (string) that is requested for display.
    *   clientId  ID of client that sent the command.
    *   title     Title of page that loaded URI (likely) corresponds to.
    *
    * The 'data' parameter to the callback will not be defined.
    *
-   * @param uri
+   * @param uris
+   *        An array containing URI objects to display
+   * @param uris[].uri
    *        String URI that was received
-   * @param clientId
+   * @param uris[].clientId
    *        ID of client that sent URI
-   * @param title
+   * @param uris[].title
    *        String title of page that URI corresponds to. Older clients may not
    *        send this.
    */
-  _handleDisplayURI: function _handleDisplayURI(uri, clientId, title) {
-    this._log.info("Received a URI for display: " + uri + " (" + title +
-                   ") from " + clientId);
-
-    let subject = {uri: uri, client: clientId, title: title};
-    Svc.Obs.notify("weave:engine:clients:display-uri", subject);
+  _handleDisplayURIs: function _handleDisplayURIs(uris) {
+    Svc.Obs.notify("weave:engine:clients:display-uris", uris);
   },
 
   _removeRemoteClient(id) {
     delete this._store._remoteClients[id];
     this._tracker.removeChangedID(id);
   },
 };