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

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/>.
  */
 
 "use strict";
 
-const {require} = chrome.extension.getBackgroundPage();
+let iframe = document.getElementById("content");
 
-document.addEventListener("DOMContentLoaded", () =>

[Manish Jethani, 2017/09/13 16:07:43]

No need for DOMContentLoaded now.

+iframe.onload = () =>
+{
+  document.title = iframe.contentDocument.title;
+};
+
+chrome.runtime.sendMessage({

[Oleksandr, 2017/10/09 11:49:59]

Edge does not support the 'chrome' namespace for extensions. We should map to
'browser' here instead.

[Manish Jethani, 2017/10/09 12:00:55]

Thanks.

[Sebastian Noack, 2017/10/09 15:09:55]

Note that we already do so in ext/common.js:
https://hg.adblockplus.org/adblockpluschrome/file/7136ad8e321d/ext/common.js#l22

+  type: "app.get",
+  what: "application"
+},
+application =>
 {
   // Load the mobile version of the options page on Firefox for Android.
-  let iframe = document.getElementById("content");
-  let src = iframe.getAttribute("data-src-" + require("info").application) ||
-            iframe.getAttribute("data-src");
-  src += location.search + location.hash;

[Manish Jethani, 2017/09/13 16:07:43]

I find myself agreeing with Sebastian now that this is not required. If it's
required in the future, it should be done properly to address the actual use
case whenever it comes up.

-  iframe.src = src;

[Thomas Greiner, 2017/09/13 14:56:29]

What difference does it make whether we do the navigation in the top-level frame
or a sub frame? Wouldn't it actually be simpler to just get rid of the iFrame
and simply do `location.href = src` here without having to first wait for
"DOMContentLoaded"?

I'd say that we should use a redirect so that pages that we link to in the
options page, by default, will open in the top-level frame rather than in the
iFrame. Some problems with loading an external page within a sub frame:
- The navigation is not reflected in the browser's address bar
- Some pages prevent that they're being shown in an iFrame (e.g. by using the
"X-Frame-Options" HTTP header)

[Manish Jethani, 2017/09/13 16:07:43]

There are two issues with redirection at the top level:

 1.  The redirection is quite visible, which is slightly annoying in terms of
user experience (but not a very big deal otherwise)
 2.  If the address bar shows anything other than "options.html",
runtime.openOptionsPage will open yet another options page when it is called,
which partly defeats the purpose of making these changes

With the iframe embedding approach, the options page is always "options.html"
regardless of what platform it is on.

As for DOMContentLoaded, we can simply move the script tag to the bottom of the
HTML instead.
If these aren't actual problems we face with this approach right now, maybe we
should keep these in mind without abandoning this approach, especially since top
level redirection has its own problems as I mentioned above.

[Sebastian Noack, 2017/09/13 16:43:24]

It seems that all (external) links in the options page are opened in a new tab
anyway. If we should ever have links that are supposed to be opened in the same
tab we can just add the "target=_top" attribute to the link (or add a <base
target="_top"> element to the document).

I generally agree with Manish, and think using an iframe is causing less
problems than redirecting the top level frame, here.

[Thomas Greiner, 2017/09/13 16:46:44]

Do you mean on Android or in general?
I see what you mean. Can't we just use `window.history.replaceState(null, null,
"/options.html")` in the two options pages to swap out the URL? Because that way
they could pretend to be "options.html" without causing a redirect (at least as
long as the origin of the new URL is the same).

[Manish Jethani, 2017/09/14 04:13:51]

The redirection is visible on both desktop and mobile, it looks especially bad
on mobile though. Basically there's a progress bar below the address bar and it
gets interrupted and reset.
I just tried this approach, and here's what I found:

 1.  Works OK on desktop (Chrome and Firefox), except for the updating of the
address bar and an extra entry in the user's browsing history
 2.  On Firefox for Android, the "app.respond" message sent to the options page
doesn't make it because of the redirection being too slow

+  iframe.src = iframe.getAttribute("data-src-" + application) ||
+               iframe.getAttribute("data-src");
 });