Noissue - Fixed browserAction in Firefox on 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
@@ skipping 159 matching lines @@
       // We use a relative URL here because of this Edge issue:
       // https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/10276332
       browser.tabs.create({url: optionsUrl}, () =>
       {
         returnShowOptionsCall(optionsTab, callback);
       });
     }
   });
 };
 
 // We need to clear the popup URL on Firefox for Android in order for the

[kzar, 2018/05/24 12:09:01]

This comment is pretty good at explaining how come we have to avoid setting the
popup URL for Android Firefox, but so far it doesn't mention why we take care to
set it in the metadata for other platforms like Chrome (so that AdBlock can
override the setting in its metadata). Perhaps you should mention that here.

[Sebastian Noack, 2018/05/24 12:37:27]

Well, setting it in the metadata.*/manifest.json file is the canonical way. Not
sure, if its worth to explicitly mention AdBlock here (or even appropriate, as
we never mention AdBlock anywhere else in our code base).

[kzar, 2018/05/24 12:58:07]

Yea fair enough. I drafted a possible improvement but deleted it again, in
fairness the existing comment is pretty good.

 // options page to open instead of the bubble. Unfortunately there's a bug[1]
 // which prevents us from doing that, so we must avoid setting the URL on
 // Firefox from the manifest at all, instead setting it here only for
 // non-mobile.
 // [1] - https://bugzilla.mozilla.org/show_bug.cgi?id=1414613
 if ("getBrowserInfo" in browser.runtime)
 {
-  browser.runtime.getBrowserInfo().then(browserInfo =>
-  {
-    if (browserInfo.name == "Fennec")
-      browser.browserAction.setPopup({popup: ""});
-  });
+  Promise.all([browser.browserAction.getPopup({}),
+               browser.runtime.getBrowserInfo()]).then(
+    ([popup, browserInfo]) =>
+    {
+      if (!popup && browserInfo.name != "Fennec")
+        browser.browserAction.setPopup({popup: "popup.html"});
+    }
+  );
 }
 
 // On Firefox for Android, open the options page directly when the browser
 // action is clicked.
 browser.browserAction.onClicked.addListener(() =>
 {
   browser.tabs.query({active: true, lastFocusedWindow: true}, ([tab]) =>
   {
     let currentPage = new ext.Page(tab);
 
     showOptions((optionsPage, port) =>
     {
       if (!/^https?:$/.test(currentPage.url.protocol))
         return;
 
       port.postMessage({
         type: "app.respond",
         action: "showPageOptions",
         args: [
           {
             host: currentPage.url.hostname.replace(/^www\./, ""),
             whitelisted: !!checkWhitelisted(currentPage)
           }
         ]
       });
     });
   });
 });