Noissue - Don't hard-code options and popup URL

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 {checkWhitelisted} = require("./whitelisting");
 const info = require("../buildtools/info");
 
 const manifest = browser.runtime.getManifest();
 const optionsUrl = manifest.options_page || manifest.options_ui.page;

[kzar, 2018/05/15 13:32:31]

I trust you got it right, but i wonder where manifest.options_page comes from. I
just checked and I see it as manifest.options_ui.page for both gecko and chrome.

[Sebastian Noack, 2018/05/15 14:17:15]

That is correct, but there is also Microsoft Edge where it is options_page.

-const popupUrl = manifest.browser_action.default_popup;

[kzar, 2018/05/15 13:32:31]

I don't see this one for gecko.

[Sebastian Noack, 2018/05/15 14:17:15]

Argh, right. We removed the popup URL from the manifest in the Firefox build.
I'm doing it the other way around now.

 
 function findOptionsTab(callback)
 {
   browser.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
@@ skipping 143 matching lines @@
 // We need to clear the popup URL on Firefox for Android in order for the
 // 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: popupUrl});
-  });
-}
-else
-{
-  browser.browserAction.setPopup({popup: popupUrl});
+    if (browserInfo.name == "Fennec")
+      browser.browserAction.setPopup({popup: ""});
+  });
 }
 
 // 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)
           }
         ]
       });
     });
   });
 });