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";
 
 let iframe = document.getElementById("content");
 
 iframe.onload = () =>
 {
-  iframe.style.visibility = "visible";
   document.title = iframe.contentDocument.title;
 };
 
 chrome.runtime.sendMessage({

[Manish Jethani, 2017/10/04 22:03:51]

After this change, we start to load the content a little bit after the top
frame's DOMContentLoaded. This causes the entire loading process to be visible
to the user: first the HTML is loaded (with no styling) [1], then the styles are
loaded. I think this looks bad and we should avoid it, especially because it's
the result of this iframe hack.

[1]: https://en.wikipedia.org/wiki/Flash_of_unstyled_content

[Manish Jethani, 2017/10/04 22:05:58]

Just to clarify, that's the point of Patch Set 22. If we show the content only
after the frame's onload, the user doesn't see the FOUC.

[Thomas Greiner, 2017/10/05 11:51:43]

This workaround seems to be only necessary due to the way desktop-options.html
is currently implemented. Therefore we should either ignore it since it's an
issue that's unrelated to the integration of mobile-options.html or at least add
a comment to clarify this fact so that we can get rid of it as soon as we
replace desktop-options.html with the new options page.

[Manish Jethani, 2017/10/05 12:14:25]

Would you say that this workaround, if it is necessary at all (since we're
moving to the new options page), rather belongs in desktop-options.html?

I noticed this problem does not occur on mobile, maybe we should drop this
workaround in that case.

[Thomas Greiner, 2017/10/05 12:34:36]

Probably, because this issue can be reproduced in the current version of the
extension we should've fixed it earlier but seems like nobody noticed it until
now.
I'd say that it's not worth fixing it anymore since we're going to get rid of
the current options page before the next release.
Yep, it's likely related to jQuery which we're not using in either the mobile or
the new options page so the workaround won't be necessary.

[Manish Jethani, 2017/10/05 13:26:30]

Done.

[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.
   iframe.src = iframe.getAttribute("data-src-" + application) ||
                iframe.getAttribute("data-src");
 });