1386316 - Fix options_ui on Android

Status: VERIFIED FIXED

(WebExtensions :: Android, defect, P1)

Description

[Eric Jung [:ericjung]]

Options are truncated.

Spawned from bug 1302504.

Screenshots:
* https://bugzilla.mozilla.org/attachment.cgi?id=8885455
* https://bugzilla.mozilla.org/attachment.cgi?id=8885752

Comment 1

[Jan Henning [:JanH]]

Attachment: options_ui on phone in portrait mode

Comment 2

[Jan Henning [:JanH]]

It's not much better on a phone, either.

Additionally, especially in portrait mode on a phone, leaving a whole column empty just to display the add-on icon a feels like a waste of space, too - approximately 15 % of the display width is left empty that way in the example just uploaded. Maybe below a certain display width, the add-on description could be left in a two-column layout with the icon on the left and the description on the right, but the options_ui then displayed across the whole page width.

And on a side note, in case anyone else notices this, the options_ui also seems to be affected by bug 1384512: Once the displayport timer expires after 15 seconds of inactivity (i.e. no scrolling, touching, ...), the options_ui expands to cover the whole of the page down to the Disable/Uninstall buttons (and might also be drawn over parts of the add-on description at the top as well).

Comment 3

[Paul [pwd]]

Options_ui simply isn't working for some addons at all. I know that Cookie AutoDelete and uBlock Origin are being forced to employ a workaround in order to get something to appear[1]. It's not working for HTTPS Everywhere either.

[1] https://github.com/mrdokenny/Cookie-AutoDelete/issues/12

Comment 4

[Jan Henning [:JanH]]

Sorry, shortened the title too much.

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1386316 - Resize Android WebExtension Options UI iframe to match its content size.

Review commit: https://reviewboard.mozilla.org/r/165672/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/165672/

Comment 6

[Jan Henning [:JanH]]

Testing with Stylus [1] on my phone it's looking better, but vertical scrolling is now rather choppy, as the iframe still is a little bit too small for its content in the vertical direction, which means that the first vertical scroll scrolls the iframe as far as possible (which is not very far) and then the second scroll scrolls the whole page.
Horizontal scrolling is fine, though - only the iframe scrolls, but not the rest of the page, so there are no weird interactions there.

[1] https://addons.mozilla.org/en-US/firefox/addon/styl-us/ - use the Desktop version of AMO to install it

Comment 7

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Jan Henning [:JanH] from comment #6)
> Testing with Stylus [1] on my phone it's looking better, but vertical
> scrolling is now rather choppy, as the iframe still is a little bit too
> small for its content in the vertical direction, which means that the first
> vertical scroll scrolls the iframe as far as possible (which is not very
> far) and then the second scroll scrolls the whole page.
> Horizontal scrolling is fine, though - only the iframe scrolls, but not the
> rest of the page, so there are no weird interactions there.
> 
> [1] https://addons.mozilla.org/en-US/firefox/addon/styl-us/ - use the
> Desktop version of AMO to install it

Thanks a lot for having started to verify the preliminary patch!

I was already suspecting that the iframe resizing logic still need some tweaks,
I'm in the process of creating a test case for the addon options 
(that unfortunately is currently missing on Firefox for Android)
and I'm working on testing (and fixing) this behavior.

I'm going to update this patch soon.

Comment 8

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8894502 [details]
Bug 1386316 - Resize Android WebExtension Options UI iframe to match its content size.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/165672/diff/1-2/

Comment 9

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8894502 [details]
Bug 1386316 - Resize Android WebExtension Options UI iframe to match its content size.

https://reviewboard.mozilla.org/r/165672/#review172890

::: mobile/android/components/extensions/test/mochitest/test_ext_options_ui.html:66
(Diff revision 2)
> +  content.window.history.back();
> +}
> +
> +add_task(async function test_options_ui_iframe_height() {
> +  function background() {
> +    browser.test.sendMessage("background-page-loaded");

is background even necessary?

Comment 10

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8894502 [details]
Bug 1386316 - Resize Android WebExtension Options UI iframe to match its content size.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/165672/diff/2-3/

Comment 11

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #9)
> Comment on attachment 8894502 [details]
> is background even necessary?

you are right, it is not actually necessary (`await extension.startup()` is already enough),
and so I removed it from the updated patch.

Comment 12

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8894502 [details]
Bug 1386316 - Resize Android WebExtension Options UI iframe to match its content size.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/165672/diff/3-4/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e5ae7beff732
Resize Android WebExtension Options UI iframe to match its content size. r=mixedpuppy

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e5ae7beff732

Comment 15

[CosminB]

Attachment: TruncatedOptions.gif

This issue is verified as fixed on Fennec 57.0a1 (2017-08-28) under Android 6.0.1.

Options are not truncated for extensions that have options_ui.

Please see the attached video.