Closed Bug 940040 Opened 6 years ago Closed 6 years ago

Pressing and releasing "Alt" opens the File menu, on Linux (when it should just reveal the menu bar)

Categories

(Core :: XUL, defect)

All
Linux
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla29

People

(Reporter: hub, Assigned: neil)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [Australis:P2][Australis:M?])

Attachments

(2 files, 1 obsolete file)

This is on Fedora 19. Running Gnome 3. This is m-c with Australis.

Press Alt
The menu pull down

Expected:
The menu shouldn't pull down. This is not the behaviour of Gtk apps.

It didn't happen before Australis.
This is a regression.
How to pull down the menu in Gtk apps?
Alt + accel.
Hardware: x86_64 → All
It has been brought to my attention that the menu bar is hidden by default. Well, it isn't here. It like it was before: on top.

Also "hidding menu bar" smells a lot like an Ubuntu-ism. Note: I run Gnome 3, not Unity, on Fedora 19.
Duplicate of this bug: 940643
(In reply to Hubert Figuiere [:hub] from comment #3)
> It has been brought to my attention that the menu bar is hidden by default.
> Well, it isn't here. It like it was before: on top.

FWIW, you can make it hidden/shown by default, if you like, via View | Toolbars | Menu Bar.

(But anyway, let's not be too concerned with discussing the merits of hiding the menu bar in this bug - let's keep it targeted on the fact that "alt" actually opens a menu, which it should not.)
One other side note: As described in (duplicate) bug 940643, Alt doesn't pull down a menu on Windows Nightly -- this bug's behavior seems linux-specific (though I haven't tested mac).

(In the Windows nightly, Alt reveals the menubar & subtly *highlights* the file menu -- indicating that if you press downarrow or enter, that's the menu that's going to pop open. I suspect we might be trying to do that subtle-highlight thing on Linux as well, but maybe our linux menus don't have a highlighted-but-not-open state, so we end up accidentally popping open the menu?)
[Copying whiteboard flags that mconley set on duplicate bug 940643]
Whiteboard: [Australis:P4][Australis:M?]
(In reply to Daniel Holbert [:dholbert] from comment #5)
> (In reply to Hubert Figuiere [:hub] from comment #3)
> > It has been brought to my attention that the menu bar is hidden by default.
> > Well, it isn't here. It like it was before: on top.
> 
> FWIW, you can make it hidden/shown by default, if you like, via View |
> Toolbars | Menu Bar.
> 
> (But anyway, let's not be too concerned with discussing the merits of hiding
> the menu bar in this bug - let's keep it targeted on the fact that "alt"
> actually opens a menu, which it should not.)

I was just mentioning it for the purpose of making sure the problem is understood and fixed. I actually didn't know it was supposed to be hidden.
FWIW, my feedback on the menu bar is in bug 940669
Blocks: 376649
Component: Keyboard Navigation → XUL
Product: Firefox → Core
Summary: Alt cause menus to pull down → Pressing and releasing "Alt" opens the File menu, on Linux (when it should just reveal the menu bar)
I don't think it should reveal the menu bar. Because doing so cause a whole "relayout" of the UI and this is really disruptive.
hub, how do you think the menu bar would be revealed, then? (when it's hidden)
(In reply to Daniel Holbert [:dholbert] from comment #11)
> hub, how do you think the menu bar would be revealed, then? (when it's
> hidden)

See bug 940669#c comment 9. F10 is the standard on Gnome.
I mean see bug 940669 comment 9
Attached patch altkeymenu (obsolete) — Splinter Review
I'm not sure why the UI changes affected this, but this is actually a regression from bug 376649. Perhaps the old UI code was hiding this bug is some way.

Could the reporter or someone verify this on a Fedora system. This patch fixes the issue on my Ubuntu build. Pressing Alt by itself should activate the menubar without opening the menu. Pressing F10 by itself should open the File menu.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #8341114 - Flags: review?(neil)
I'm not running Fedora, but I can confirm that the patch fixes the issue on my Ubuntu 13.10 (w/ gnome-shell) system.

(BTW: /layout/xul/base/src has recently been flattened into /layout/xul, so to make the patch apply on current m-c, just open it in an editor and search-and-replace "/base/src/" with "/".)
Attached patch Possible patchSplinter Review
I was looking at this, and I couldn't help thinking that attachment 8341114 [details] [diff] [review] was over-engineered; I came up with this code which I feel is short and to the point.
Attachment #8351258 - Flags: review?(enndeakin)
Comment on attachment 8351258 [details] [diff] [review]
Possible patch

Yes, this is much better.
Attachment #8351258 - Flags: review?(enndeakin) → review+
Attachment #8341114 - Attachment is obsolete: true
Attachment #8341114 - Flags: review?(neil)
OK, so what's happening is that I'm briefly activating the file menu before opening the popup, which wasn't happening before. Is this likely to be a problem i.e. should I just fix the test to expect the new behaviour or do we need to go back to the original patch?
I expect bug 941640 depends on this, and bug 941640 is P3, so this should probably be P3, too.

(In reply to neil@parkwaycc.co.uk from comment #20)
> OK, so what's happening is that I'm briefly activating the file menu before
> opening the popup, which wasn't happening before. Is this likely to be a
> problem i.e. should I just fix the test to expect the new behaviour or do we
> need to go back to the original patch?

I can't answer this question, but Neil, do you know?
Flags: needinfo?(enndeakin)
Whiteboard: [Australis:P4][Australis:M?] → [Australis:P3][Australis:M?]
I don't think it would be a problem, as long as the DOMMenuItemActive event isn't being sent multiple times and that the menu isn't being activated, then deactivated and the reactivated again.

Perhaps someone from the accessibility team could test just to be sure.
Flags: needinfo?(enndeakin) → needinfo?(surkov.alexander)
Comment on attachment 8351258 [details] [diff] [review]
Possible patch

(In reply to Neil Deakin from comment #22)
> I don't think it would be a problem, as long as the DOMMenuItemActive event
> isn't being sent multiple times and that the menu isn't being activated,
> then deactivated and the reactivated again.
> 
> Perhaps someone from the accessibility team could test just to be sure.

right, a11y relies on these events so if we don't receive them then we should be ok but to be on safe side we need to ask Marco to check it.
Attachment #8351258 - Flags: feedback?(marco.zehe)
Flags: needinfo?(surkov.alexander)
Whiteboard: [Australis:P3][Australis:M?] → [Australis:P2][Australis:M?]
(In reply to Neil Deakin from comment #22)
> I don't think it would be a problem, as long as the DOMMenuItemActive event
> isn't being sent multiple times and that the menu isn't being activated,
> then deactivated and the reactivated again.
The sequence is
* menubar DOMMenuBarActive
* menubar menu DOMMenuItemActive (new)
* menubar menu popup popupshowing
* menubar menu popup menuitem DOMMenuItemActive
So there are now two DOMMenuItemActive events, one for the first menu itself, as well as one for the first item on that menu.
Interestingly this is the sequence of events that I recorded for an Alt+F keypress on Windows:
* menubar DOMMenuItemActive
* menubar menu menupopup popupshowing
* menubar menu DOMMenuItemActive
* menubar menu menupopup menuitem DOMMenuItemActive
It should be ok. I notice the failing tests are for F10 keypresses though. It looks like we don't have a Linux Alt key alone test. One should be added after the windows one.
(In reply to Neil Deakin from comment #27)
> It should be ok. I notice the failing tests are for F10 keypresses though.
> It looks like we don't have a Linux Alt key alone test. One should be added
> after the windows one.

Shouldn't the Alt key alone now have the same results as on Windows, so I can just remove the relevant conditions?
Sure. I assume if the events are sent the same in that case.
Comment on attachment 8361127 [details] [diff] [review]
Test changes

Should be ok, although I thought you meant that the second "DOMMenuItemActive filemenu" was now not present, which is more correct.
Attachment #8361127 - Flags: review?(enndeakin) → review+
https://hg.mozilla.org/mozilla-central/rev/357883f2205b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Comment on attachment 8351258 [details] [diff] [review]
Possible patch

Yup, nothing broken, all still good. :)
Attachment #8351258 - Flags: feedback?(marco.zehe) → feedback+
Depends on: 961672
This is a huge improvement, thanks.  An uncommon but supported method of moving windows around in linux is to hold alt and left click anywhere on the window (resize is alt+right click).  I don't think those shortcuts are super common, but I wanted to note that this patch, while a huge improvement over what was there, still does make the menu appear and shift all the content down when moving or resizing the window.  If I had a good suggestion I'd file another bug, but I don't. :)
Depends on: 962007
Note that I suspect bug 962007 to be a regression from this bug. Marked dependency.
This patch was backed out of Holly for causing bug 961672. This patch was never intended to land on Holly anyway, since this is an Australis bug.

Back out was: https://hg.mozilla.org/projects/holly/rev/45677214a968
(In reply to Wil Clouser [:clouserw] from comment #35)
> This is a huge improvement, thanks.  An uncommon but supported method of
> moving windows around in linux is to hold alt and left click anywhere on the
> window (resize is alt+right click).  I don't think those shortcuts are super
> common, but I wanted to note that this patch, while a huge improvement over
> what was there, still does make the menu appear and shift all the content
> down when moving or resizing the window.  If I had a good suggestion I'd
> file another bug, but I don't. :)

I was encouraged to file another bug, so discussion for this should go in bug 962135. Thanks.
No longer depends on: 962007
A quick "ALT" press/release shows the Ubuntu search bar, but I guess that's ok due to some system settings ?
Flags: needinfo?
Flags: needinfo? → needinfo?(neil)
Sorry, I have no idea about Ubuntu, I only have Fedora.
Flags: needinfo?(neil)
(In reply to Paul Silaghi, QA [:pauly] from comment #39)
> A quick "ALT" press/release shows the Ubuntu search bar, but I guess that's
> ok due to some system settings ?
Holding just a little longer the "ALT" key triggers then the Firefox menu.
I'm considering this verified fixed, if anyone has any objection please say.
Reproduced nightly 2014-01-10.
Verified fixed nightly 29.0a1 2014-01-21, ubuntu 13.04 x64.
Status: RESOLVED → VERIFIED
This still is not the same behaviour as it used to be. Pressing alt should show the menu, but releasing it, should make it go away. A single press on alt still moves keyboard input to the menu, which is annoying.
I believe the current behavior you're describing is actually what's intended (and matches how apps with hidden-by-default menubars behave on Windows, at least. I'm not sure there is precedent for apps with hidden-by-default menubars on Linux).

If it annoys you, you can always make the menubar permanently visible by default -- though actually, I just noticed that even then, pressing & releasing "alt" will focus the menubar, when I don't think it should. I filed bug 966683 on that.
I think we are our own precedent :)

I have just tested it on release, and with the menubar deactivated, it does *nothing*. And I like it that way. Firefox shouldn't do things that disturb me when pressing one button. I absolutely hate one button shortcuts for that reason, and especially if they're part of another shortcut (which I might start, but not finish, leaving me pressing a completely unrelated shortcut). Hide/show as I proposed (and though was default, but apparently I was wrong) would imo be the best solution. You don't need the somewhat more obscure F10 shortcut to view the menu, and at the same time, accident presses don't move focus.

Making the menubar permanent by default is a band-aid I don't like. It gives me an extra, useless row in Firefox.
(In reply to Timvde from comment #44)
> I think we are our own precedent :)
> 
> I have just tested it on release, and with the menubar deactivated, it does
> *nothing*. And I like it that way. Firefox shouldn't do things that disturb
> me when pressing one button. I absolutely hate one button shortcuts for that
> reason, and especially if they're part of another shortcut (which I might
> start, but not finish, leaving me pressing a completely unrelated shortcut).
> Hide/show as I proposed (and though was default, but apparently I was wrong)
> would imo be the best solution. You don't need the somewhat more obscure F10
> shortcut to view the menu, and at the same time, accident presses don't move
> focus.

If keyup of alt hid the menubar again, how would you navigate the menu? Arrow keys while holding alt? That seems clearly inferior if not downright cumbersome (plus, there are shortcuts on Linux that use alt+arrows, from what I'm told!). I don't think what you're suggesting makes sense.
Please note: this bug is *solely* about the Alt key *opening* the File menu, which is incorrect whether or not you believe the Alt key should *activate* the menubar or not, discussion of which belongs elsewhere. Thank you.
You need to log in before you can comment on or make changes to this bug.