Issue 5587, 5748 - Use mobile options page on Firefox for Android

lib/options.js

+/*
+ * This file is part of Adblock Plus <https://adblockplus.org/>,
+ * Copyright (C) 2006-present eyeo GmbH
+ *
+ * Adblock Plus is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * Adblock Plus is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/** @module options */
+
+"use strict";
+
+const {getDecodedHostname} = require("url");
+const {checkWhitelisted} = require("whitelisting");
+const {port} = require("messaging");
+const info = require("info");
+
+const optionsUrl = "options.html";
+
+function findOptionsTab(callback)
+{
+  chrome.tabs.query({}, tabs =>
+  {
+    // We find a tab ourselves because Edge has a bug when quering tabs with
+    // extension URL protocol:
+    // https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/8094141/
+    // https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/8604703/
+    // Firefox won't let us query for moz-extension:// pages either, though
+    // starting with Firefox 56 an extension can query for its own URLs:
+    // https://bugzilla.mozilla.org/show_bug.cgi?id=1271354
+    let fullOptionsUrl = ext.getURL(optionsUrl);
+    callback(tabs.find(element => element.url == fullOptionsUrl));
+  });
+}
+
+function returnShowOptionsCall(optionsTab, callback)
+{
+  if (!callback)
+    return;
+
+  if (optionsTab)
+  {
+    callback(new ext.Page(optionsTab));
+  }
+  else
+  {
+    // If we don't already have an options page, it means we've just opened
+    // one, in which case we must find the tab, wait for it to be ready, and
+    // then return the call.
+    findOptionsTab(tab =>
+    {
+      if (!tab)
+      {
+        // This likely means that there was an error. Let the callback deal
+        // with it.
+        callback();

[Manish Jethani, 2017/10/02 08:03:14]

I think it's better to call the callback anyway with no page object, then let it
deal with it (either explicitly ignore the error or let it crash), but let me
know if you disagree.

[Sebastian Noack, 2017/10/04 01:22:32]

I can think of two scenarios here:

1. chrome.runtime.openOptionsPage() or chrome.tabs.create() failed. This is a
rather theoretical scenario, which I cannot see how it could happen other than a
browser bug (or bug in our code here). Moreover, there is nothing the calling
code could do about it. Also note that chrome.runtime.lastError wouldn't be set
accordingly in this case, since the last API called is chrome.tabs.query().
2. The user (or another extension) closed the options page again before
chrome.tabs.query() completed. In this case doing nothing is the right thing to
do, and either causing an error to be logged or requiring the calling code to
check for the argument seems unnecessary.

[Manish Jethani, 2017/10/04 03:46:49]

In general I think that a callback should be called even if there's an error,
because there's often some cleanup code in callbacks. If this were a promise,
for instance, we would reject it here, but with no "reason" argument. There is
one instance where we'd want the callback to be called even if everything else
failed, i.e. in popup.js when the Options menu item is clicked, otherwise the UI
would seem unresponsive.

Anyway, like you said, this is theoretical. I've removed the call to the
callback now.

[Sebastian Noack, 2017/10/04 05:33:14]

This would rather be an argument to return a Promise here. So that code that
wouldn't handle the error anyway, could just register a success callback and in
the unlikely case of an error it would just get propagated up the promise chain,
or logged if this is the end of the chain. But still, if the options page just
gets closed before this code is reached, this isn't an error, but any further
processing would lead to one. Anyway, I think what we have now is good enough.

[Manish Jethani, 2017/10/04 13:02:45]

Acknowledged.

+        return;
+      }
+
+      function onMessage(message, sender)
+      {
+        if (message.type == "app.listen" &&
+            sender.page && sender.page.id == tab.id)
+        {
+          port.off("app.listen", onMessage);
+          callback(new ext.Page(tab));
+        }
+      }
+
+      port.on("app.listen", onMessage);
+    });
+  }
+}
+
+let showOptions =
+/**
+ * Opens the options page.
+ *
+ * @param {function} callback
+ */
+exports.showOptions = callback =>
+{
+  findOptionsTab(optionsTab =>
+  {
+    // Edge does not yet support runtime.openOptionsPage (tested version 38)
+    if ("openOptionsPage" in chrome.runtime &&
+        // Some versions of Firefox for Android before version 57 do have a
+        // runtime.openOptionsPage but it doesn't do anything.
+        // https://bugzilla.mozilla.org/show_bug.cgi?id=1364945
+        (info.application != "fennec" ||
+         parseInt(info.applicationVersion, 10) >= 57))
+    {
+      chrome.runtime.openOptionsPage(() =>
+      {
+        returnShowOptionsCall(optionsTab, callback);
+      });
+    }
+    else if (optionsTab)
+    {
+      // Firefox for Android before version 57 does not support
+      // runtime.openOptionsPage, nor does it support the windows API.
+      // Since there is effectively only one window on the mobile browser,
+      // there's no need to bring it into focus.
+      if ("windows" in chrome)
+        chrome.windows.update(optionsTab.windowId, {focused: true});
+
+      chrome.tabs.update(optionsTab.id, {active: true});
+
+      returnShowOptionsCall(optionsTab, callback);
+    }
+    else
+    {
+      // We use a relative URL here because of this Edge issue:
+      // https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/10276332
+      chrome.tabs.create({url: optionsUrl}, () =>
+      {
+        returnShowOptionsCall(optionsTab, callback);
+      });
+    }
+  });
+};
+
+// On Firefox for Android, open the options page directly when the browser
+// action is clicked.
+chrome.browserAction.onClicked.addListener(() =>
+{
+  ext.pages.query({active: true, lastFocusedWindow: true}, pages =>
+  {
+    let currentPage = pages[0];
+
+    showOptions(optionsPage =>
+    {
+      if (!["http:", "https:"].includes(currentPage.url.protocol))

[Sebastian Noack, 2017/10/04 01:22:33]

You could just use a regular expression here (if you don't want to duplicate the
right-hand side, which would be fine too). If you are concerned, about
performance this shouldn't be any worse than creating an array on demand.

[Manish Jethani, 2017/10/04 03:46:49]

Done.

+        return;
+
+      optionsPage.sendMessage({
+        type: "app.respond",
+        action: "showPageOptions",
+        args: [
+          {
+            host: getDecodedHostname(currentPage.url).replace(/^www\./, ""),
+            whitelisted: !!checkWhitelisted(currentPage)
+          }
+        ]
+      });
+    });
+  });
+});