Fix options_ui on Android

VERIFIED FIXED in Firefox 57

Status

P1
normal
VERIFIED FIXED
a year ago
2 months ago

People

(Reporter: ericjung, Assigned: rpl)

Tracking

(Blocks: 1 bug)

unspecified
mozilla57
Dependency tree / graph

Firefox Tracking Flags

(firefox57 verified)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Reporter)

Updated

a year ago
Depends on: 1302504
Created attachment 8894268 [details]
options_ui on phone in portrait mode
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).
Summary: Fix options_ui on Android tablets → Fix options_ui

Comment 3

a year ago
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
Sorry, shortened the title too much.
Summary: Fix options_ui → Fix options_ui on Android
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Assignee: nobody → lgreco
Status: NEW → ASSIGNED
Priority: -- → P1
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
(Assignee)

Comment 7

a year ago
(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 hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8894502 - Flags: review?(mixedpuppy)

Comment 9

a year ago
mozreview-review
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?
Attachment #8894502 - Flags: review?(mixedpuppy) → review+
Comment hidden (mozreview-request)
(Assignee)

Comment 11

a year ago
(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 hidden (mozreview-request)

Comment 13

a year ago
Pushed by luca.greco@alcacoop.it:
https://hg.mozilla.org/integration/autoland/rev/e5ae7beff732
Resize Android WebExtension Options UI iframe to match its content size. r=mixedpuppy

Comment 14

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e5ae7beff732
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57

Comment 15

a year ago
Created attachment 8901791 [details]
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.

Updated

a year ago
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified

Updated

2 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.