Issue 5347 - Open options page on Firefox for Android when browser action is clicked

ext/background.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 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
@@ skipping 301 matching lines @@
   chrome.tabs.onActivated.addListener(details =>
   {
     ext.pages.onActivated._dispatch(new Page({id: details.tabId}));
   });
 
 
   /* Browser actions */
 
   // On Firefox for Android, open the options page directly when the browser
   // action is clicked.
   if (!("getPopup" in chrome.browserAction))

[Manish Jethani, 2017/08/16 00:44:45]

We're checking for browserAction.getPopup, i.e. whether the popup is supported.
If there's no popup, we open the options page directly.

   {
     chrome.browserAction.onClicked.addListener(() =>
     {
       ext.showOptions();
     });
   }
 
   let BrowserAction = function(tabId)
   {
     this._tabId = tabId;
@@ skipping 335 matching lines @@
     },
     remove(key, callback)
     {
       chrome.storage.local.remove(key, callback);
     },
     onChanged: chrome.storage.onChanged
   };
 
   /* Options */
 
   ext.showOptions = callback =>

[Manish Jethani, 2017/08/16 00:44:44]

Note that now it's one function with the if...else moved inside. We have to do
this because info.application becomes available only later.

Also require isn't available at the time of defining this function, therefore we
have to call it within the function body.

   {
-    let {application} = require("info");
-
     if ("openOptionsPage" in chrome.runtime &&
         // Firefox for Android does have a runtime.openOptionsPage but it
         // doesn't do anything.
-        application && application != "unknown" &&
-        application != "fennec")

[Wladimir Palant, 2017/08/17 12:21:21]

This value comes from browser's internal data, not from user agent parsing or
such. So you can actually rely on it saying "fennec" for Firefox Mobile, no need
to test the other values.

[Manish Jethani, 2017/08/17 13:21:26]

This information is fetched asynchronously, it may not be available when this
function is called.

https://hg.adblockplus.org/buildtools/file/ef4ad61830eb/templates/geckoInfo.j...

[Sebastian Noack, 2017/08/18 10:13:47]

The functionality here is triggered by user interaction, and it seems
practically impossible that the user triggers the option page through the user
interface before initialization of info.application completes. On the other hand
there are potentially other scenarios (e.g. Chromium based browsers with quirky
user agent strings) where it will be set to "unknown".

[Manish Jethani, 2017/08/18 14:20:30]

This can be mitigated by adding an additional check for info.platform == "gecko"

Anyway, the worst that could happen is that it's a no-op on Android, that's no
so bad especially because the chance of it happening is so miniscule.

I've made the change.

+        // https://bugzilla.mozilla.org/show_bug.cgi?id=1364945
+        require("info").application != "fennec")
     {
       if (!callback)
       {
         chrome.runtime.openOptionsPage();
       }
       else
       {
         chrome.runtime.openOptionsPage(() =>
         {
           if (chrome.runtime.lastError)

[Manish Jethani, 2017/08/18 14:20:30]

Shouldn't this still call the callback even though there's an error?

In general the caller should be allowed to deal with the error (reset state,
update the UI, etc.). It's not a problem in this particular case though.

[Wladimir Palant, 2017/08/18 21:03:35]

Yes, we could easily do that if we start returning promises here. As things are
now, the caller is only expecting to be notified on success.

             return;
 
           chrome.tabs.query({active: true, lastFocusedWindow: true}, tabs =>
           {
             if (tabs.length > 0)
             {
               if (tabs[0].status == "complete")
                 callback(new Page(tabs[0]));
               else
                 afterTabLoaded(callback)(tabs[0]);
@@ skipping 16 matching lines @@
         let queryInfo = {url: optionsUrl};
 
         // extension pages can't be accessed in incognito windows. In order to
         // correctly mimic the way in which Chrome opens extension options,
         // we have to focus the options page in any other window.
         if (!win.incognito)
           queryInfo.windowId = win.id;
 
         chrome.tabs.query(queryInfo, tabs =>
         {
           if (tabs && tabs.length > 0)

[Manish Jethani, 2017/08/16 00:44:44]

Firefox returns null instead of an empty array.

           {
             let tab = tabs[0];
 
             chrome.windows.update(tab.windowId, {focused: true});
             chrome.tabs.update(tab.id, {active: true});
 
             if (callback)
               callback(new Page(tab));
           }
           else
           {
             ext.pages.open(optionsUrl, callback);
           }
         });
       });
     }
   };
 
   /* Windows */
   ext.windows = {
     create(createData, callback)
     {
       chrome.windows.create(createData, createdWindow =>
       {
         afterTabLoaded(callback)(createdWindow.tabs[0]);
       });
     }
   };
 }());