1625600 - panelview with long title is rendered incorrectly, oversized and painting issues

Status: ASSIGNED

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

Description

[Rob Wu [:robwu]]

Attachment: repro-plus-devtools-preview.mp4

(forked from bug 1624238 comment 15 and comment 24 onwards)

STR:

1. Create a directory, and a file called manifest.json with the following content:

{
    "name": "Long button title",
    "version": "1",
    "manifest_version": 2,
    "browser_action": {
        "default_area": "menupanel",
        "default_popup": "manifest.json",
        "default_title": "abcdefghijklmnopqrstuvwxyz0123456789_abcdefghijklmnopqrstuvwxyz0123456789_abcdefghijklmnopqrstuvwxyz0123456789"
    }
}

2. Visit about:debugging and load the extension from step 1.
3. Click on the "More tools..." button next to the hamburger menu.
4. Click on the menu item from the extension, i.e. "abcdefghijklmnopqrstuvwxyz0123456789_abcd..."
5. Look at the opened panel.

Expected:

• It should be small and the content aligned at the right.

Actual:

• The width of the panel is excessively large, and has a blank space at the right. There are severe rendering issues (for another example, see https://bugzilla.mozilla.org/show_bug.cgi?id=1624238#c15). I have witnessed several errors in which the panel is drawn, with varying degrees of shifting and clipping.

I have attached a video to demonstrate the issue. At the end I have also showed some interaction from the browser toolbox to reveal the relevant underlying DOM tree.

In bug 1624238 I mentioned that the bug disappears when overflow:hidden is removed from the panelview. It is declared at https://searchfox.org/mozilla-central/rev/b712398b7fae54ef377a558d6f16dede7a7f8530/browser/themes/shared/customizableui/panelUI.inc.css#349-351

Comment 1

[:Gijs (he/him)]

Attachment: Wide-panel

I don't really see any graphics corruption on Windows, fwiw. That seems like a separate graphics bug...

Comment 2

[:Gijs (he/him)]

Attachment: Bug 1625600 - remove superfluous overflow:hidden for extension panelviews to resolve issues with long pane titles, r?kmag

From some testing on Windows, I can't figure out why we need this styling,
and re-reading what's left of the reviews after mozreview's demise has not
helped. So perhaps we can just remove it?

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/96614a3debc3
remove superfluous overflow:hidden for extension panelviews to resolve issues with long pane titles, r=kmag

Comment 4

[Narcis Beleuzu [:NarcisB]]

Backed out for bc failure on browser_ext_browserAction_popup_resize_bottom

Backout link: https://hg.mozilla.org/integration/autoland/rev/5e6e96236725fdc53fd01ca8da4e08223e35ab44
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=296223391&repo=autoland&lineNumber=1709

Comment 5

[:Gijs (he/him)]

Can't reproduce the failure locally; my first trypush was broken ( bug 1628377), new attempt: https://treeherder.mozilla.org/#/jobs?repo=try&revision=54f625c87ed7541b6d614c881426cd9a72929e24

Comment 6

[:Gijs (he/him)]

The test failures are to do with the subview panel overlapping the top bar on ubuntu, which I guess is why it doesn't reproduce on Windows? Still not 100% sure why things are wrong - this code is really hairy and timing-sensitive and hard to debug. :-(

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:Gijs, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 8

[:Gijs (he/him)]

This is a pain to debug. There's some kind of race condition with the panel sizing on Linux and I haven't been able to figure it out because I have higher priority things to work on.

Comment 9

[:Gijs (he/him)]

Luca, any chance you could help me here, as it seems you worked on https://searchfox.org/mozilla-central/source/browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize_bottom.js ? I think the resizing logic doesn't properly take into account the height of the title above the subview, or something - but it only results in failure on linux...

Comment 10

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

(In reply to :Gijs (he/him) from comment #9)

Luca, any chance you could help me here, as it seems you worked on https://searchfox.org/mozilla-central/source/browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize_bottom.js ? I think the resizing logic doesn't properly take into account the height of the title above the subview, or something - but it only results in failure on linux...

Sure, I'll take a look asap (I'm not clearing the needinfo yet, so that I will not forget to come back to this)

Comment 11

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

Attachment: panel_before_and_after.png

(In reply to :Gijs (he/him) from comment #9)

Luca, any chance you could help me here, as it seems you worked on https://searchfox.org/mozilla-central/source/browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize_bottom.js ? I think the resizing logic doesn't properly take into account the height of the title above the subview, or something - but it only results in failure on linux...

I finally took a look today, and the failure is definitely because the panel header is now overlapping with the top bar (which doesn't seem to be happening without the small change to the css applied to the attached patch)

The attached screenshot shows the panel opened in the test case before (on the left) and after(on the right) the change from the attached patch.

The amount of overlapping looks exactly the height of the panel header, I'm wondering how that wasn't triggering the issue, it doesn't look that we are retrieving the height of the .panel-header element in the resize login from ExtensionPopups.jsm.

I guess that one way of dealing with it may be to take the .panel-header height into account in the resizeBrowser method if the height computed here is going to make the panel to overlap to the top or bottom panels.