Closed
Bug 1377793
Opened 4 years ago
Closed 4 years ago
Hamburger menu dropdown is truncated, no scrollbar
Categories
(Firefox :: Toolbars and Customization, defect, P1)
Tracking
()
VERIFIED
FIXED
Firefox 56
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | + | verified |
firefox56 | - | verified |
People
(Reporter: alice0775, Assigned: Paolo)
References
Details
(Keywords: regression)
Attachments
(3 files)
170.78 KB,
image/png
|
Details | |
59 bytes,
text/x-review-board-request
|
Gijs
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
59 bytes,
text/x-review-board-request
|
Gijs
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
[Tracking Requested - why for this release]: UI regression Reproducible: 55.0beta only (At present, Nightly 56 is a photon structure, so there is a possibility that it is not affected) Steps To Reproduce: 1. Place many toolbarbutton ino Hamburger menu 2. (if necessary) Reduce browser height and position so the hamburger menu dropdown will be overflowed 3. Open hamburger menu Actual Results: Hamburger menu dropdown is truncated, no scrollbar. Unable to access some buttons(Customize/Help/Exit button etc...) Expected Results: Hamburger menu dropdown should appear vertical scrollbar
![]() |
Reporter | |
Comment 1•4 years ago
|
||
regression-window |
Regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=90956bd266a83c70d81fd7806b5d660fb0afba10&tochange=2d16236446afaceb84c2d09c0d8e1056c8001ffd Regressed by: 2d16236446af Paolo Amadini — Bug 1009116 - Redo resizing architecture of panelmultiview. r=Gijs
Blocks: 1009116
Flags: needinfo?(paolo.mozmail)
![]() |
Reporter | |
Comment 2•4 years ago
|
||
![]() |
Reporter | |
Comment 3•4 years ago
|
||
And I can also reproduce the problem on Nightly56.0a1(2017-07-02) if set browser.photon.structure.enabled to false
Updated•4 years ago
|
Whiteboard: [photon-structure] [triage]
Comment 4•4 years ago
|
||
I don't think we need to track this as part of Photon (so I'm removing the whiteboard flags), but we should track for 55 and either fix or back out bug 1009116 for 55 (and potentially 56).
Whiteboard: [photon-structure] [triage]
Assignee | ||
Comment 5•4 years ago
|
||
I'll take a look, this is more likely a result of CSS changes than of the way we do the resizing.
Assignee | ||
Comment 7•4 years ago
|
||
So, it turns out this is another case of box-in-block layout issues. I have a patch that works around this by using box layout when the height of the view exceeds the available space. This was possibly one of the reasons, or maybe the only reason, why the overflow listeners were present in the previous architecture. While ideally this should be handled by the platform layout code, now at least this workaround is marked as such and is self-contained in a few lines.
Flags: needinfo?(paolo.mozmail)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 9•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f37ac4f1c07bd6900622787cb2ca070afcf7ce80
Assignee | ||
Updated•4 years ago
|
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Priority: -- → P1
Comment 10•4 years ago
|
||
mozreview-review |
Comment on attachment 8883361 [details] Bug 1377793 - Fix scrolling in the non-Photon main menu. https://reviewboard.mozilla.org/r/154252/#review159568 This looks pretty hacky, but it seems to work OK in my testing. My only nit is that this breaks whimsy (empty your panel). I don't think that should stop us shipping this though, given it's all going away with Photon.
Attachment #8883361 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 11•4 years ago
|
||
(In reply to :Gijs from comment #10) > My only nit is that this breaks whimsy (empty your panel). That's the weirdest unexplained consequence of a CSS change... I thought everything was fine, and someone intentionally changed the animation to have the whimsycorn charge towards the right side of the panel! Anyways, I realized that I removed a property that was necessary in customize mode to keep the entire area as a drag-and-drop target, and restoring it fixed the animation as well, for some reason.
Comment hidden (mozreview-request) |
Comment 13•4 years ago
|
||
Pushed by paolo.mozmail@amadzone.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/4cb18ada2641 Fix scrolling in the non-Photon main menu. r=Gijs
Comment 14•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4cb18ada2641
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Comment 15•4 years ago
|
||
Hi Alice, Can you help check if this is fixed in latest nightly?
Flags: needinfo?(alice0775)
![]() |
Reporter | |
Comment 16•4 years ago
|
||
[Tracking Requested - why for this release]: not fixed No, not fixed. The problem is still reproduced on the nightly56.0a1[1]. [1] https://hg.mozilla.org/mozilla-central/rev/018b3829d0a7f6944a5ae1489f3a64c9e2893909 Mozilla/5.0 (Windows NT 10.0; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0 ID:20170706030206 set browser.photon.structure.enabled to false in about:config Restart Nightly and then follow STR of comment#0
Status: RESOLVED → REOPENED
tracking-firefox56:
--- → ?
Flags: needinfo?(alice0775)
Resolution: FIXED → ---
Assignee | ||
Comment 17•4 years ago
|
||
Eh, unfortunately this works on Mac and Linux, but on Windows we don't get the actual main view height until after the panel is shown, even using getBoundingClientRect(). I've tried the "popuppositioned" event but it's still too early. I tried to use the "popupshown" event, but then the jump between the view states is very visible and occurs every time the panel is opened, so I don't think it's viable. I wonder how the original code could handle this. Maybe the overflow listener is called after the popup opens and before it is painted, worth checking. Maybe it's just a side effect of looking for the height continuously during the panel opening animation. Gijs, any other ideas on how to move forward? If the dedicated overflow listener doesn't work, the safest bet might be to back out bug 1009116 in its entirety, although this would reintroduce a number of issues we had in the other panels.
Flags: needinfo?(gijskruitbosch+bugs)
Comment 18•4 years ago
|
||
(In reply to :Paolo Amadini from comment #17) > I tried to use the "popupshown" event, but then the jump between the view > states is very visible and occurs every time the panel is opened, so I don't > think it's viable. Is there a jump even when the panel has enough room not to overflow any parts of it? > I wonder how the original code could handle this. Maybe the overflow > listener is called after the popup opens and before it is painted, worth > checking. Maybe it's just a side effect of looking for the height > continuously during the panel opening animation. I don't know. The comment in the patch that landed here feels like it's the type of problem that would be solvable with enough modern flexbox. It's not entirely clear to me from the patch why the max-height isn't enough anyway. Potentially relevant: bug 1373968.
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(paolo.mozmail)
Assignee | ||
Comment 19•4 years ago
|
||
(In reply to :Gijs from comment #18) > Is there a jump even when the panel has enough room not to overflow any > parts of it? No, only when it overflows. > I don't know. The comment in the patch that landed here feels like it's the > type of problem that would be solvable with enough modern flexbox. It's not > entirely clear to me from the patch why the max-height isn't enough anyway. If platform did the right thing when mixing -moz-box and block/flex layout, we wouldn't have this issue...
Flags: needinfo?(paolo.mozmail)
Assignee | ||
Comment 20•4 years ago
|
||
(In reply to :Paolo Amadini from comment #19) > (In reply to :Gijs from comment #18) > > Is there a jump even when the panel has enough room not to overflow any > > parts of it? > > No, only when it overflows. Actually, there was no magic recipe in the old code, and this jump simply already existed before bug 1009116 landed. We can just use the "popupshown" event on Windows, and probably we improved the situation for Mac and Linux by using the "popupshowing" event.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•4 years ago
|
Attachment #8883361 -
Attachment is obsolete: true
Comment hidden (mozreview-request) |
Assignee | ||
Updated•4 years ago
|
Attachment #8883361 -
Attachment is obsolete: false
Comment 23•4 years ago
|
||
mozreview-review |
Comment on attachment 8884643 [details] Bug 1377793 - Fix scrolling in the non-Photon main menu on Windows. https://reviewboard.mozilla.org/r/155498/#review160658
Attachment #8884643 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment 24•4 years ago
|
||
Pushed by paolo.mozmail@amadzone.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/33d81aa58bca Fix scrolling in the non-Photon main menu on Windows. r=Gijs
Comment 25•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/33d81aa58bca
Status: REOPENED → RESOLVED
Closed: 4 years ago → 4 years ago
Resolution: --- → FIXED
Comment 26•4 years ago
|
||
Please request Beta approval on this when you get a chance.
Flags: needinfo?(paolo.mozmail)
Assignee | ||
Comment 27•4 years ago
|
||
Alice, can you confirm this is now fixed on Windows too? I'll request uplift to Beta once it's verified.
Flags: needinfo?(alice0775)
![]() |
Reporter | |
Comment 28•4 years ago
|
||
(In reply to :Paolo Amadini from comment #27) > Alice, can you confirm this is now fixed on Windows too? I'll request uplift > to Beta once it's verified. I confirmed that Nightly56.0a1(2017-07-11) fixed the problem with the photon structure off.
Flags: needinfo?(alice0775)
Assignee | ||
Comment 29•4 years ago
|
||
Comment on attachment 8883361 [details] Bug 1377793 - Fix scrolling in the non-Photon main menu. Approval Request Comment [Feature/Bug causing the regression]: Bug 1009116 [User impact if declined]: Main application menu cut off on very low resolutions or when the menu button is vertically in the middle of the screen. [Is this code covered by automated tests?]: There are no tests for this particular issue. The issue does not affect the existing panel tests. [Has the fix been verified in Nightly?]: [Needs manual test from QE? If yes, steps to reproduce]: Yes. If testing on Nightly, the Photon structure preference should be disabled. Move the window so that the menu button is vertically in the middle of the screen and check that a scrollbar appears in the main application menu. Customize more buttons into the menu if the resolution is too high for this to happen. Should be tested on all platforms separately. See also comment 0. [List of other uplifts needed for the feature/fix]: There are two patches to uplift in this bug. [Is the change risky?]: Low risk [Why is the change risky/not risky?]: Only applies to the specific case tested here. [String changes made/needed]: None
Flags: needinfo?(paolo.mozmail)
Attachment #8883361 -
Flags: approval-mozilla-beta?
Assignee | ||
Updated•4 years ago
|
Attachment #8884643 -
Flags: approval-mozilla-beta?
Comment 30•4 years ago
|
||
Comment on attachment 8883361 [details] Bug 1377793 - Fix scrolling in the non-Photon main menu. fix for hamburger menu in beta55, should be in 55.0b9
Attachment #8883361 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•4 years ago
|
Attachment #8884643 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 31•4 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/a598927cab98 https://hg.mozilla.org/releases/mozilla-beta/rev/f06fa691cdfd
Updated•4 years ago
|
Flags: qe-verify+
Comment 32•4 years ago
|
||
I managed to reproduce the issue on Firefox 56.0a1(2017-07-02), under Windows 10x64. The issue is no longer reproducible on Firefox 55.0b12, or on Firefox 56.0a1 (2017-07-26), under Windows 10x64 or under Ubuntu 16.04x64. On Firefox 55.0b12, under macOS 10.12.1 the scrollbar is not displayed for the hamburger menu. Note that Firefox 56.0a1 (2017-07-26), under macOS, the scrollbar is displayed even if the browser.photon.structure.enabled pref is set to true or false. Should I reopen this bug for this problem, or it would be better to open a new one?
Flags: needinfo?(gijskruitbosch+bugs)
Comment 33•4 years ago
|
||
(In reply to Mihai Boldan, QA [:mboldan] from comment #32) > I managed to reproduce the issue on Firefox 56.0a1(2017-07-02), under > Windows 10x64. > The issue is no longer reproducible on Firefox 55.0b12, or on Firefox 56.0a1 > (2017-07-26), under Windows 10x64 or under Ubuntu 16.04x64. > > On Firefox 55.0b12, under macOS 10.12.1 the scrollbar is not displayed for > the hamburger menu. > > Note that Firefox 56.0a1 (2017-07-26), under macOS, the scrollbar is > displayed even if the browser.photon.structure.enabled pref is set to true > or false. > > Should I reopen this bug for this problem, or it would be better to open a > new one? Please file a new bug and needinfo Paolo. I expect we may want this to track 55. :-(
Flags: needinfo?(gijskruitbosch+bugs)
Comment 35•4 years ago
|
||
Logged Bug 1384899 for the issue described in Comment 32. Since another bug is covering the issue reproducible on macOS, I'm marking this issue Verified Fixed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(mihai.boldan)
Updated•4 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•