Closed Bug 1384899 Opened 7 years ago Closed 7 years ago

Scrollbar is not available in the hamburger menu

Categories

(Firefox :: Toolbars and Customization, defect)

55 Branch
All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
firefox55 - fixed
firefox56 --- unaffected

People

(Reporter: mboldan, Assigned: Paolo)

References

Details

(Keywords: regression)

Attachments

(1 file)

[Note]:
- Logged based on Bug 1377793 Comment 33

[Affected versions]:
- Firefox 55.0b12

[Affected platforms]:
- macOS 10.12.1

[Steps to reproduce]:
1. Launch Firefox.
2. Go to Customization mode and add a large no. of buttons in the hamburger menu.
3. Exit Customization mode.
4. Reduce browser height and position so the hamburger menu dropdown will be overflowed.
5. Open hamburger menu.

[Expected result]:
- A scrollbar is displayed and all the buttons are accessible. 

[Actual result]:
- No scrollbar is available and only the first buttons from the hamburger menu are accessible.
- Here are some screenshots of the issue: https://i.imgur.com/82na9VK.png and https://i.imgur.com/5hLrQU9.jpg

[Regression range]:
- This is a regression. I will investigate asap.

[Additional notes]:
- This issue is not reproducible on Windows 10x64, or on Ubuntu 16.04x64.
- It's not reproducible on Firefox Latest Nightly even if the  browser.photon.structure.enabled pref is set on true or on false.
Paolo, I could have sworn that we tested this in bug 1377793 and it worked then (and the flags on this bug indicate it's working on 56?). What changed? Are we missing a dep patch on 55 or something? :-(
Blocks: 1377793
Flags: needinfo?(paolo.mozmail)
Eh, I'll try to debug a Beta build to find out what's happening.
In Firefox 55 on Mac we have to delay measuring the main view height, just like we do on Windows. This obviously causes the panel to jump when opening it, but this is not a regression compared to the old behavior.

For Firefox 56 I don't think we have to do anything, because there is no issue and the code will be inactive in Firefox 57 anyways. If this code was active for longer, I'd be wary of regressions due to changes in platform behavior, but as things stand we might just want to test manually that the workaround still works after 56 moves to Beta.
Flags: needinfo?(paolo.mozmail)
Comment on attachment 8890848 [details]
Bug 1384899 - Fix missing scrollbar in the main menu on Mac.

https://reviewboard.mozilla.org/r/162070/#review167306

r=me for the code change, but I haven't independently checked that this fixes the issue. Perhaps it'd be worth a beta trypush to get QA to check that this fixes things for them, too?
Attachment #8890848 - Flags: review?(gijskruitbosch+bugs) → review+
Ah, should have pushed to try from the command line when I was still on the Beta branch :-)

The Autoland try build is supposed to be here, but is slow to appear:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba4f13c449a8
That may be bug 1384934, and no luck with a push-to-try right now. Sounds like the try build will have to wait tomorrow.
Comment on attachment 8890848 [details]
Bug 1384899 - Fix missing scrollbar in the main menu on Mac.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1009116
[User impact if declined]: On Mac only in Firefox 55, the main application menu would be 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?]: No
[Has the fix been verified in Nightly?]: This should be verified in Beta. We're also waiting on a try build blocked on infra issues, so I'm requesting approval to speed things up in the meantime. Will land on Beta once the try build is ready and verified.
[Needs manual test from QE? If yes, steps to reproduce]: See comment 0.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Low risk
[Why is the change risky/not risky?]: Changes are limited to the feature, and we're just enabling on Mac a safer code path we already used on Windows.
[String changes made/needed]: None
Attachment #8890848 - Flags: approval-mozilla-beta?
Some TreeHerder results seem to be unblocked now, but I don't see the Mac build yet.

Mihai, can you test the builds once they are ready? It would be good to verify Windows and Linux also.
Flags: needinfo?(mihai.boldan)
Comment on attachment 8890848 [details]
Bug 1384899 - Fix missing scrollbar in the main menu on Mac.

hamburger menu fix for osx, should be in 55.0b13
Attachment #8890848 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee: nobody → paolo.mozmail
(In reply to :Paolo Amadini from comment #9)
> Some TreeHerder results seem to be unblocked now, but I don't see the Mac
> build yet.
> 
> Mihai, can you test the builds once they are ready? It would be good to
> verify Windows and Linux also.

The scrollbar is correctly displayed on Firefox 55.0b13 and on Firefox 56.0a1(2017-07-26), under Mac OS X 10.11.
I've also managed to verify the issue on Firefox 55.0b13 and on Firefox 56.0a1(2017-07-27), under Windows 10x64 and under Ubuntu 16.04x64 and I noticed that only on Latest Nightly build and only under Ubuntu OS, if the browser.photon.structure.enabled pref is set to false, the scrollbar is not available in the hamburger menu. 

Here is the regression range of the issue described above:
Last good revision: 388d81ed93fa640f91d155f36254667c734157cf
First bad revision: f1693d664f8e8ee4c79801630c181c28095cad56
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=388d81ed93fa640f91d155f36254667c734157cf&tochange=f1693d664f8e8ee4c79801630c181c28095cad56

Should I reopen a new issue for this problem?
Flags: needinfo?(mihai.boldan)
(In reply to Mihai Boldan, QA [:mboldan][PTO 1-15Aug] from comment #12)
> Should I reopen a new issue for this problem?

I'd say we should wait for Firefox 56 to move to Beta and test all platforms there. This will be the last version with Photon disabled, and it's better to test the complete non-Photon configuration than switching the preference manually, as the results may be different.
Does this reproduce on 56 now that it's moved to beta?
Flags: needinfo?(mihai.boldan)
(In reply to :Gijs from comment #14)
> Does this reproduce on 56 now that it's moved to beta?

The scorllbar is correctly displayed in Firefox 56.0b3, under Mac OS X 10.12.5.
Please let me know if I can help any further.
Flags: needinfo?(mihai.boldan)
Closing this out then, ni -> Paolo to confirm there's nothing left to do here (I don't think so).
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(paolo.mozmail)
Resolution: --- → FIXED
Yep, I think we're good here.
Flags: needinfo?(paolo.mozmail)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: