Bug 1244597 - Show notification on incoming tab. r=markh draft
authorEdouard Oger <eoger@fastmail.com>
Thu, 07 Jul 2016 13:33:29 -0700
changeset 388750 90c352d2d16118eadf79715ebd59e4c89b0e6c9a
parent 388749 711963e8daa312ae06409f8ab5c06612cb0b8f7b
child 525592 5b20f5f484b073ef12a9399e9fe03e4f7a6f97e4
push id23226
push userbmo:edouard.oger@gmail.com
push dateSun, 17 Jul 2016 08:27:34 +0000
reviewersmarkh
bugs1244597
milestone50.0a1
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
services/sync/tests/unit/test_clients_engine.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)
@@ -2431,34 +2431,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
@@ -33,8 +33,23 @@ syncStartNotification.body2 = %S will be
 # 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 (sendTabToAllDevices.menuitem)
 # Displayed in the Send Tabs context menu when right clicking a tab, a page or a link.
 sendTabToAllDevices.menuitem = All Devices
+
+# 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
@@ -378,16 +378,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;
@@ -400,23 +401,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.
    *
@@ -476,42 +481,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);
   },
 };
 
--- a/services/sync/tests/unit/test_clients_engine.js
+++ b/services/sync/tests/unit/test_clients_engine.js
@@ -761,24 +761,24 @@ add_test(function test_receive_display_u
     command: "displayURI",
     args: [uri, remoteId, title],
   };
 
   engine.localCommands = [command];
 
   // Received 'displayURI' command should result in the topic defined below
   // being called.
-  let ev = "weave:engine:clients:display-uri";
+  let ev = "weave:engine:clients:display-uris";
 
   let handler = function(subject, data) {
     Svc.Obs.remove(ev, handler);
 
-    do_check_eq(subject.uri, uri);
-    do_check_eq(subject.client, remoteId);
-    do_check_eq(subject.title, title);
+    do_check_eq(subject[0].uri, uri);
+    do_check_eq(subject[0].clientId, remoteId);
+    do_check_eq(subject[0].title, title);
     do_check_eq(data, null);
 
     run_next_test();
   };
 
   Svc.Obs.add(ev, handler);
 
   do_check_true(engine.processIncomingCommands());