1251981 - Firefox menus and panels overlap Windows Taskbar

Status: RESOLVED FIXED

(Firefox :: Menus, defect)

Description

[arni2033]

>>>   My Info:   Win7_64, Nightly 47, 32bit, ID 20160225030209
STR_1:
1. Dock Windows Taskbar to the bottom side of the screen
2. Maximize Firefox window
3. Open the following "data:" url
>   data:text/html,<div><style>div{width:40px; height:40px; position:fixed; left:0px; bottom:10px; background:gray;}
4. Hover mouse over the gray rectangle. Right-click.
5. Repeat until you get context menu opened to the bottom:
 Move mouse pointer 10px to the up, then right-click again.

STR_2:
1. Dock Windows Taskbar to the right side of the screen
2. Maximize Firefox window
3. Open the following "data:" url
>   data:text/html,<div><style>div{width:40px; height:40px; position:fixed; right:10px; top:10px; background:black;}
4. Hover mouse over the black rectangle. Right-click.

AR:  Context menu overlaps taskbar
ER:  Context menu shouldn't overlap taskbar, because that's default behavior on Windows.

Comment 3

[Neil Deakin]

You can set the preference 'ui.menusCanOverlapOSBar' to 0 as a workaround.

But we can change the default setting here in widget/windows/nsLookAndFeel.cpp

Comment 4

[japandi]

Attachment: Bug 1251981 - Don't allow context menus to overlap Windows taskbar by default, and fix window_largemenu.xhtml test to match. r?NeilDeakin

Comment 5

[japandi]

Submitted patch: https://phabricator.services.mozilla.com/D289514

Comment 6

[japandi]

"Hi, the patch has been approved with r+ by gstoll. Could someone with commit access please trigger autoland? Thank you!

Comment 7

[Kelly Cochrane [:kcochrane]]

:japandi, we can potentially land this if it's even still an issue, but this is not something we marked as a good first bug.. It's touching a .cpp file which our team rarely if ever works with. We'll need to test to make sure that it's still an issue a decade later.

Comment 8

[japandi]

Hi Kelly, I tested the reproduction steps from the bug report on Windows 11 with the latest Firefox Nightly. The context menu no longer overlaps the Windows taskbar, it stops correctly above it. Screenshot attached. The bug appears to still be present in the code (the default value is still 1) but Windows 11 may be handling it differently. Happy to test further if needed.

Comment 9

[Stephen Thompson [:sthompson]]

A reviewer in the #win-reviewer group that approved your patch can queue your patch to land if they're happy to accept it.

Comment 10

[Neil Deakin]

Note that the change here afftects all popups/panels, not just context menus.

Comment 11

[japandi]

Hi Neil, thank you for the feedback. I noticed that macOS (Cocoa), GTK, UIKit, and Headless all already return 0 for MenusCanOverlapOSBar and work fine with all popup types. Should I be concerned about the impact on non-context-menu popups on Windows specifically? Happy to adjust the patch if needed.

Comment 12

[japandi]

can you take a look into my patch and potentially land it thank you.

Comment 13

[japandi]

I need this bug to be reviewed please, I need to know the guidance so that I can move forward, please.

Comment 14

[:Gijs (he/him)]

(In reply to japandi from comment #13)

I need this bug to be reviewed please, I need to know the guidance so that I can move forward, please.

Neil should review the patch.

(In reply to Neil Deakin from comment #10)

Note that the change here afftects all popups/panels, not just context menus.

Is that not what other apps do? I tried quickly with notepad yesterday, on win11 that doesn't overlap the taskbar even with the main menus.

My only reservation here is that there's a chance this is going to increase the number of situations (ie screen size / density combinations) in which the main hamburger menu scrolls, on smaller screens. Neil, can you provide feedback on the patch, consulting with UX as needed? Thank you.

Comment 15

[:Gijs (he/him)]

For comment #14.

Comment 16

[Neil Deakin]

No, that seems fine. I only comment because the summary of the bug only mentions context menus.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/996d3397b86a
https://hg.mozilla.org/integration/autoland/rev/b7d6a645ea40
Don't allow context menus to overlap Windows taskbar by default. r=NeilDeakin,win-reviewers,gstoll

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/93e33621cbfd
https://hg.mozilla.org/integration/autoland/rev/937f9b82b039
Revert "Bug 1251981 - Don't allow context menus to overlap Windows taskbar by default. r=NeilDeakin,win-reviewers,gstoll" for causing multiple failures at largemenu.html

Comment 19

[Silaghi Andreea]

Backout for causing multiple failures at largemenu.html
push with failure
failure log
backout link

Comment 20

[japandi]

redirecting to a more experienced engineer.

Comment 21

[:Gijs (he/him)]

(In reply to japandi from comment #20)

redirecting to a more experienced engineer.

This is your patch - it got backed out because it broke tests. To get it to land, you'd need to figure out why the test broke and whether that means there should be changes to your patch or whether the test needs updating (or a bit of both). The previous comments have links to more details of what broke; try looking at those, maybe running the test locally with/without your patch and seeing what behaves differently, or adding logging and debugging on try. If you have specific questions, ask for help from the mentor for this bug.

Comment 22

[Neil Deakin]

The failing test has a manual reference to assume that Windows doesn't overlap the OS bar.

https://searchfox.org/firefox-main/source/toolkit/content/tests/chrome/window_largemenu.xhtml#446

The test may just work by removing that assumption, now that the behaviour has changed.

But you should try running the test locally to verify.

Comment 23

[japandi]

Attachment: Bug 1251981 - Fix window_largemenu.xhtml test to not assume Windows can overlap OS chrome. r?NeilDeakin

Comment 24

[japandi]

I made an update to the patch, please let me know if I am right or wrong.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/383b22df89b7
https://hg.mozilla.org/integration/autoland/rev/290346c649c4
Don't allow context menus to overlap Windows taskbar by default, and fix window_largemenu.xhtml test to match. r=NeilDeakin,win-reviewers,gstoll

Comment 26

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/290346c649c4