1403466 - Extension popups in the overflow menu are cropped from the bottom

Status: VERIFIED FIXED

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

Description

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

Attachment: screenshot

Spotted on macOS Yosemite with Firefox 57.0b3.

STR:
1. Run Firefox.
2. Install e.g. uBlock origin.
3. Pin the add-on icon to the overflow menu.
4. Open the overflow menu.
5. Click the uBlock origin item.
6. The opened popup has blank space on the bottom overlapping the extension popup.

The customize toolbars footer is not hidden (only its text).

Comment 1

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

Note this does not happen for the system icon, like library.

Comment 3

[:Gijs (out for now; he/him)]

Reopening this because content is actually cut off here, and that's not what got reported in bug 1402845.

I can't reproduce this, though it's clear that other people can, cf. bug 1406029.

Can you find better STR? I see content being cut off (with the steps from bug 1406029), but not this light-grey bar at the bottom. On Windows, I can't reproduce any issues with uBO. :-\

Comment 4

[:Gijs (out for now; he/him)]

Ni for comment #3.

Comment 5

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

These STR are the best I have. During a workshop yesterday, I have reprorduced this with 6 more people and used this bug to demonstrate mozregression. I will add the found regression window once I get back go my computer in an hour or so.

Note this does not affect uBO only, the same happens with Niobium Password Generator too.

Comment 6

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

Here is the regression window/commit https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=3300c15011d3a6661aaf8eee254b693d7e99f392&tochange=5b700335fc30f2faf8aa12c9e38bc05e6c1e5e22.

Comment 7

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

It's reproducible on Mac OS X and Fedora, but I think during the workshop people saw it on Windows too.

But I guess it happens only when the opened popup resizes (is created dynamically by JavaScript) or is bigger than the overflow menu popup, because for example Nightly Tester Tools WebExtension popup (which is small) is not affected.

Comment 8

[u603568]

I could reproduce with uBlock origin 1.14.14 and Nightly.

Using the browser toolbox.

The stack element, a direct child of the panelview element, and which contains the extension popup panel, has a height of 386px. This is correct.

However, the panelview parent element has a max-height of 386px. This is incorrect, since this does not account for the box.panel-header element, another child of the panelview element. The incorrectly cropped height seems to match the height of the panel header. It seems to me the fix is simply to take into account the panel header height when setting the max-height style of the panelview element -- or maybe more generally the sum of the height of all direct children of the panelview element.

Comment 9

[Mike de Boer [:mikedeboer]]

Attachment: Bug 1403466 - Don't set a max-height on panelviews used by WebExtensions, because it causes browser contents to be cut-off.

Setting a max-height caused the '_handleDOMChange' method in ext-browser-content.js
to consistently lie about the scrollHeight, since it was never allowed to grow
beyond the maxHeight - even when the document needs to be larger to fit its contents.
We don't need this aggressiveness in Photon panels anyway, so that makes it
doubly safe to remove this code.

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

Comment 10

[:Gijs (out for now; he/him)]

Comment on attachment 8916682 [details]
Bug 1403466 - Don't set a max-height on panelviews used by WebExtensions, because it causes browser contents to be cut-off.

https://reviewboard.mozilla.org/r/187772/#review192826

Comment 11

[Mike de Boer [:mikedeboer]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8126d13f7be94cacae8e4415b76a8d2a937f5ace

Comment 12

[Mozilla Autoland]

hg error in cmd: hg rebase -s 25f7ed092e34 -d e3f39de40209: abort: can't rebase public changeset 25f7ed092e34
(see 'hg help phases' for details)

Comment 13

[Mike de Boer [:mikedeboer]]

Comment on attachment 8916682 [details]
Bug 1403466 - Don't set a max-height on panelviews used by WebExtensions, because it causes browser contents to be cut-off.

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

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd62fb50fe61
Don't set a max-height on panelviews used by WebExtensions, because it causes browser contents to be cut-off. r=Gijs

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 18

[Mike de Boer [:mikedeboer]]

Comment on attachment 8916682 [details]
Bug 1403466 - Don't set a max-height on panelviews used by WebExtensions, because it causes browser contents to be cut-off.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1374749, buggy WebExtension panel sizing code.
[User impact if declined]: The panel will be shown too small to be able to accommodate the full height requested by the extension, causing it to look cut off.
[Is this code covered by automated tests?]: yes, no regressions in panel sizing code found.
[Has the fix been verified in Nightly?]: No.
[Needs manual test from QE? If yes, steps to reproduce]: Yes, STR can be found in comment 0.
[List of other uplifts needed for the feature/fix]: n/a.
[Is the change risky?]: Minor risk.
[Why is the change risky/not risky?]: Because the patch/ fix is isolated to WebExtension panel handling code and doesn't touch other areas.
[String changes made/needed]: n/a.

Comment 19

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

I can verify this as working in Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0 ID:20171010220102.

Comment 20

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8916682 [details]
Bug 1403466 - Don't set a max-height on panelviews used by WebExtensions, because it causes browser contents to be cut-off.

Photon polish, Beta57+

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c609f1155d16

Comment 22

[Emil Ghitta, QA [:emilghitta]]

I have reproduced the issue mentioned in comment 0 using an affected Firefox 58.0a1 build (BuildId:20170927100120).

I have verified that the issue is not reproducible using Firefox 57.0b8 (Build Id:20171009192146) on Windows 10 64bit, macOS 10.11.6 and Ubuntu 16.04 64bit.