Open Bug 1372635 Opened 7 years ago Updated 2 years ago

browser_ext_browserAction_popup_resize.js doesn't work properly when its panelview is loaded into a smaller panel

Categories

(Firefox :: Toolbars and Customization, defect, P5)

defect

Tracking

()

Tracking Status
firefox57 --- wontfix

People

(Reporter: Gijs, Unassigned)

References

Details

browser_ext_browserAction_popup_resize.js resizes webextension panel contents to various sizes, and then checks that the resulting panel also resizes accordingly. It does this in various configurations. One of those is when the panel is in the panel menu.

In Photon, the panel menu is effectively the overflow panel. By default, that panel is empty. In non-photon, it's the old "main" hamburger panel, which is very much not empty.

When switching this test to be photon compatible in bug 1354109 I noticed it broke when actually flipping the photon pref, because of sizing issues.

It seems like the test implicitly depends on the size of the panel in which the view is contained. In particular, I believe this means that in non-photon, if you were to update the test such that it emptied out the main menupanel (save for its own button), it would fail, because while the panel is resized to accommodate the larger height, in one of the tests the width of the contents is also increased, leading to a horizontal scrollbar, for which the panel resizing code does not compensate, thus leading to the panel also having a vertical scrollbar.

I don't know if/how this is fixable outside of the test, but given it's a pre-existing issue I don't think it needs to block enabling Photon on nightly. However, given it is more likely to happen with the overflow panel than with the main menupanel, ideally we should find a way to fix it before releasing 57. I'd be interested if people have ideas about how to do this.
Component: WebExtensions: General → Toolbars and Customization
Product: Toolkit → Firefox
Summary: browser_ext_browserAction_popup_resize.js doesn't work properly when its panelview is part of a smaller panels → browser_ext_browserAction_popup_resize.js doesn't work properly when its panelview is loaded into a smaller panel
See Also: → 1372837
Padding is taken into account for height calculation already, so this doesn't seem like it'll be related to bug 1372837 / bug 1370083.
See Also: 1372837
(In reply to :Gijs from comment #0)
> ideally we should find a way to fix it before releasing 57.

Can you add the right tracking bugs or triage flags? I had this on my radar but I'm not looking into this soon, so it's better to track this if we need to.
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to :Paolo Amadini from comment #2)
> (In reply to :Gijs from comment #0)
> > ideally we should find a way to fix it before releasing 57.
> 
> Can you add the right tracking bugs or triage flags? I had this on my radar
> but I'm not looking into this soon, so it's better to track this if we need
> to.

Well, while the test would break without the workaround we put in place, the non-automated-test general version of this problem is basically just bug 1373490, which I don't think we have a usable fix for.

I don't know how likely it will be for webextensions to have overly wide panels, and for those to then end up in the panel. If anything we're reducing the likelihood of this happening by removing the overlapping bar on the left with photon. I don't know if/how the problem of double scrollbars here is solvable for webextensions generally... if it's not clear how we intend to fix this, I don't think tracking it is useful. Did you have specific ideas already?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(paolo.mozmail)
We might have to account for the preferred horizontal size of the extension panel in some way, but it may be overly complicated, and as you said it's likely the same as bug 1373490. Should we mark that one as NEW?
Flags: needinfo?(paolo.mozmail)
(In reply to :Paolo Amadini from comment #4)
> We might have to account for the preferred horizontal size of the extension
> panel in some way, but it may be overly complicated, and as you said it's
> likely the same as bug 1373490. Should we mark that one as NEW?

TBH, I expect that to be a duplicate, but also my understanding is that the webextension team does not intend to change something there, but maybe there's a misunderstanding? I'll ping them on the bug.
Priority: -- → P5
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.