Bug 1359861 - allow reader-mode tabs to be shared. r=Gijs,skhamis

Differential Revision: https://phabricator.services.mozilla.com/D170186

browser/base/content/browser-context.inc

@@ -116,7 +116,7 @@
             data-l10n-id="main-context-menu-link-send-to-device"
             hidden="true">
             <menupopup id="context-sendlinktodevice-popup"
-                  onpopupshowing="gSync.populateSendTabToDevicesMenu(event.target, gContextMenu.linkURL, gContextMenu.linkTextStr);"/>
+                  onpopupshowing="gSync.populateSendTabToDevicesMenu(event.target, gContextMenu.linkURI, gContextMenu.linkTextStr);"/>
       </menu>
       <menuseparator id="context-sep-sendlinktodevice" class="sync-ui-item"
                 hidden="true"/>
@@ -266,7 +266,7 @@
             data-l10n-id="main-context-menu-send-to-device"
             hidden="true">
         <menupopup id="context-sendpagetodevice-popup"
-                   onpopupshowing="(() => { gSync.populateSendTabToDevicesMenu(event.target, gBrowser.currentURI.spec, gBrowser.contentTitle); })()"/>
+                   onpopupshowing="(() => { gSync.populateSendTabToDevicesMenu(event.target, gBrowser.currentURI, gBrowser.contentTitle); })()"/>
       </menu>
       <menu id="fill-login" hidden="true">
         <menupopup id="fill-login-popup" />

browser/base/content/browser-sync.js

@@ -657,7 +657,7 @@ var gSync = {
   showSendToDeviceView(anchor) {
     PanelUI.showSubView("PanelUI-sendTabToDevice", anchor);
     let panelViewNode = document.getElementById("PanelUI-sendTabToDevice");
-    this.populateSendTabToDevicesView(panelViewNode);
+    this._populateSendTabToDevicesView(panelViewNode);
   },
 
   showSendToDeviceViewFromFxaMenu(anchor) {
@@ -689,11 +689,11 @@ var gSync = {
     this.emitFxaToolbarTelemetry("sync_tabs_sidebar", panel);
   },
 
-  populateSendTabToDevicesView(panelViewNode, reloadDevices = true) {
+  _populateSendTabToDevicesView(panelViewNode, reloadDevices = true) {
     let bodyNode = panelViewNode.querySelector(".panel-subview-body");
     let panelNode = panelViewNode.closest("panel");
     let browser = gBrowser.selectedBrowser;
-    let url = browser.currentURI.spec;
+    let uri = browser.currentURI;
     let title = browser.contentTitle;
     let multiselected = gBrowser.selectedTab.multiselected;
 
@@ -701,7 +701,7 @@ var gSync = {
     // changes.
     this.populateSendTabToDevicesMenu(
       bodyNode,
-      url,
+      uri,
       title,
       multiselected,
       (clientId, name, clientType, lastModified) => {
@@ -759,7 +759,7 @@ var gSync = {
       // device, and is waiting for it to show up.
       this.refreshFxaDevices().then(_ => {
         if (!window.closed) {
-          this.populateSendTabToDevicesView(panelViewNode, false);
+          this._populateSendTabToDevicesView(panelViewNode, false);
         }
       });
     }
@@ -933,8 +933,8 @@ var gSync = {
   enableSendTabIfValidTab() {
     // All tabs selected must be sendable for the Send Tab button to be enabled
     // on the FxA menu.
-    let canSendAllURIs = gBrowser.selectedTabs.every(t =>
-      BrowserUtils.isShareableURL(t.linkedBrowser.currentURI)
+    let canSendAllURIs = gBrowser.selectedTabs.every(
+      t => !!BrowserUtils.getShareableURL(t.linkedBrowser.currentURI)
     );
 
     PanelMultiView.getViewNode(
@@ -1273,12 +1273,18 @@ var gSync = {
 
   populateSendTabToDevicesMenu(
     devicesPopup,
-    url,
+    uri,
     title,
     multiselected,
     createDeviceNodeFn,
     isFxaMenu = false
   ) {
+    uri = BrowserUtils.getShareableURL(uri);
+    if (!uri) {
+      // log an error as everyone should have already checked this.
+      this.log.error("Ignoring request to share a non-sharable URL");
+      return;
+    }
     if (!createDeviceNodeFn) {
       createDeviceNodeFn = (targetId, name, targetType, lastModified) => {
         let eltName = name ? "menuitem" : "menuseparator";
@@ -1309,7 +1315,7 @@ var gSync = {
           targets,
           fragment,
           createDeviceNodeFn,
-          url,
+          uri.spec,
           title,
           multiselected,
           isFxaMenu
@@ -1533,7 +1539,7 @@ var gSync = {
     for (let tab of aTargetTab.multiselected
       ? gBrowser.selectedTabs
       : [aTargetTab]) {
-      if (BrowserUtils.isShareableURL(tab.linkedBrowser.currentURI)) {
+      if (BrowserUtils.getShareableURL(tab.linkedBrowser.currentURI)) {
         hasASendableURI = true;
         break;
       }
@@ -1584,7 +1590,7 @@ var gSync = {
       : contextMenu.browser.currentURI;
     const enabled =
       !this.sendTabConfiguredAndLoading &&
-      BrowserUtils.isShareableURL(targetURI);
+      BrowserUtils.getShareableURL(targetURI);
     const hideItems = this.shouldHideSendContextMenuItems(enabled);
 
     contextMenu.showItem(

browser/base/content/browser.js

@@ -4730,7 +4730,7 @@ let gShareUtils = {
     if (AppConstants.platform == "win") {
       // We disable the item on Windows, as there's no submenu.
       // On macOS, we handle this inside the menupopup.
-      shareURL.hidden = !BrowserUtils.isShareableURL(browser.currentURI);
+      shareURL.hidden = !BrowserUtils.getShareableURL(browser.currentURI);
     }
   },
 
@@ -4785,9 +4785,12 @@ let gShareUtils = {
     let urlToShare = null;
     let titleToShare = null;
 
-    if (browser && BrowserUtils.isShareableURL(browser.currentURI)) {
-      urlToShare = browser.currentURI;
-      titleToShare = browser.contentTitle;
+    if (browser) {
+      let maybeToShare = BrowserUtils.getShareableURL(browser.currentURI);
+      if (maybeToShare) {
+        urlToShare = maybeToShare;
+        titleToShare = browser.contentTitle;
+      }
     }
     return { urlToShare, titleToShare };
   },

browser/base/content/main-popupset.inc.xhtml

@@ -62,7 +62,7 @@
           data-lazy-l10n-id="tab-context-send-tabs-to-device"
           data-l10n-args='{"tabCount": 1}'>
       <menupopup id="context_sendTabToDevicePopupMenu"
-                 onpopupshowing="gSync.populateSendTabToDevicesMenu(event.target, TabContextMenu.contextTab.linkedBrowser.currentURI.spec, TabContextMenu.contextTab.linkedBrowser.contentTitle, TabContextMenu.contextTab.multiselected);"/>
+                 onpopupshowing="gSync.populateSendTabToDevicesMenu(event.target, TabContextMenu.contextTab.linkedBrowser.currentURI, TabContextMenu.contextTab.linkedBrowser.contentTitle, TabContextMenu.contextTab.multiselected);"/>
     </menu>
     <menu id="context_reopenInContainer"
           data-lazy-l10n-id="tab-context-open-in-new-container-tab"

browser/base/content/test/sync/head.js

@@ -20,7 +20,11 @@ function setupSendTabMocks({
     status: state,
     syncEnabled: true,
   });
-  sandbox.stub(BrowserUtils, "isShareableURL").returns(isSendableURI);
+  if (isSendableURI) {
+    sandbox.stub(BrowserUtils, "getShareableURL").returnsArg(0);
+  } else {
+    sandbox.stub(BrowserUtils, "getShareableURL").returns(null);
+  }
   sandbox.stub(fxAccounts.device, "refreshDeviceList").resolves(true);
   sandbox.stub(fxAccounts.commands.sendTab, "send").resolves({ failed: [] });
   return sandbox;

services/sync/modules/engines/tabs.sys.mjs

@@ -34,6 +34,7 @@ ChromeUtils.defineESModuleGetters(lazy, {
 
 XPCOMUtils.defineLazyModuleGetters(lazy, {
   NimbusFeatures: "resource://nimbus/ExperimentAPI.jsm",
+  ReaderMode: "resource://gre/modules/ReaderMode.jsm",
   TabsStore: "resource://gre/modules/RustTabs.jsm",
 });
 
@@ -377,6 +378,10 @@ export const TabProvider = {
               !lazy.TABS_FILTERED_SCHEMES.has(Services.io.extractScheme(url));
 
         let url = tab.linkedBrowser.currentURI?.spec;
+        // Special case for reader mode.
+        if (url && url.startsWith("about:reader?")) {
+          url = lazy.ReaderMode.getOriginalUrl(url);
+        }
         // We ignore the tab completely if the current entry url is
         // not acceptable (we need something accurate to open).
         if (!acceptable(url)) {

services/sync/tests/unit/test_tab_provider.js

@@ -42,4 +42,11 @@ add_task(async function test_getAllTabs() {
   tabs = await provider.getAllTabs(true);
   _("Ordered: " + JSON.stringify(tabs));
   equal(tabs[0].lastUsed > tabs[1].lastUsed, true);
+
+  // reader mode URLs are provided.
+  provider.getWindowEnumerator = mockGetWindowEnumerator.bind(this, [
+    "about:reader?url=http%3A%2F%2Ffoo.com%2F",
+  ]);
+  tabs = await provider.getAllTabs(true);
+  equal(tabs[0].urlHistory[0], "http://foo.com/");
 });

toolkit/modules/BrowserUtils.sys.mjs

@@ -12,6 +12,10 @@ ChromeUtils.defineESModuleGetters(lazy, {
   Region: "resource://gre/modules/Region.sys.mjs",
 });
 
+XPCOMUtils.defineLazyModuleGetters(lazy, {
+  ReaderMode: "resource://gre/modules/ReaderMode.jsm",
+});
+
 XPCOMUtils.defineLazyPreferenceGetter(
   lazy,
   "INVALID_SHAREABLE_SCHEMES",
@@ -222,18 +226,24 @@ export var BrowserUtils = {
     return uri.asciiHost || uri.spec;
   },
 
-  isShareableURL(url) {
+  // Given a URL returns a (possibly transformed) URL suitable for sharing, or null if
+  // no such URL can be obtained.
+  getShareableURL(url) {
     if (!url) {
-      return false;
+      return null;
     }
 
+    // Carve out an exception for about:reader.
+    if (url.spec.startsWith("about:reader?")) {
+      url = Services.io.newURI(lazy.ReaderMode.getOriginalUrl(url.spec));
+    }
     // Disallow sharing URLs with more than 65535 characters.
     if (url.spec.length > 65535) {
-      return false;
+      return null;
     }
     // Use the same preference as synced tabs to disable what kind
     // of tabs we can send to another device
-    return !lazy.INVALID_SHAREABLE_SCHEMES.has(url.scheme);
+    return lazy.INVALID_SHAREABLE_SCHEMES.has(url.scheme) ? null : url;
   },
 
   /**

toolkit/modules/tests/xpcshell/test_BrowserUtils.js

@@ -222,7 +222,7 @@ add_task(async function test_shouldShowCookieBannersPromo() {
   Preferences.resetBranch("browser.promo.cookiebanners");
 });
 
-add_task(function test_isShareableURL() {
+add_task(function test_getShareableURL() {
   // Some test suites, specifically android, don't have this setup properly -- so we add it manually
   if (!Preferences.get("services.sync.engine.tabs.filteredSchemes")) {
     Preferences.set(
@@ -231,22 +231,32 @@ add_task(function test_isShareableURL() {
     );
   }
   // Empty shouldn't be sendable
-  Assert.ok(!BrowserUtils.isShareableURL(""));
+  Assert.ok(!BrowserUtils.getShareableURL(""));
   // Valid
-  Assert.ok(
-    BrowserUtils.isShareableURL(Services.io.newURI("https://mozilla.org"))
-  );
+  let good = Services.io.newURI("https://mozilla.org");
+  Assert.ok(BrowserUtils.getShareableURL(good).equals(good));
   // Invalid
   Assert.ok(
-    !BrowserUtils.isShareableURL(Services.io.newURI("file://path/to/pdf.pdf"))
+    !BrowserUtils.getShareableURL(Services.io.newURI("file://path/to/pdf.pdf"))
   );
 
   // Invalid
   Assert.ok(
-    !BrowserUtils.isShareableURL(
+    !BrowserUtils.getShareableURL(
       Services.io.newURI(
         "data:application/json;base64,ewogICJ0eXBlIjogIm1haW4i=="
       )
     )
   );
+
+  // Reader mode:
+  if (AppConstants.platform !== "android") {
+    let readerUrl = Services.io.newURI(
+      "about:reader?url=" + encodeURIComponent("http://foo.com/")
+    );
+    Assert.equal(
+      BrowserUtils.getShareableURL(readerUrl).spec,
+      "http://foo.com/"
+    );
+  }
 });