Closed Bug 261715 Opened 16 years ago Closed 9 years ago

Alt+click sometimes focuses menubar (keyrepeated Alt shouldn't trigger menubar focusing behavior)

Categories

(Core :: XUL, defect, minor)

x86
Windows XP
defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla2.0b8

People

(Reporter: jruderman, Assigned: masayuki)

References

Details

Attachments

(2 files, 1 obsolete file)

Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20040911 Firefox/0.10

Steps to reproduce:
1. Start holding Alt.
2. Click (a link or a blank spot on a page).
3. Wait two seconds.
4. Release Alt.

Result: Menubar gets focus.

Because step 3 is necessary to reproduce this bug, I think this bug involves
keyrepeat with the Alt key.  Maybe an onkeypress needs to be changed to an
onkeydown.
Assignee: firefox → aaronleventhal
Status: NEW → ASSIGNED
Attachment #169403 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #169403 - Flags: review?(bryner)
This patch works around bug 91592.  It would be better to fix bug 91592, but it
would probably be harder.
This patch works no matter what we decide to do with bug 91592. I'd like us to
fix bug 91592 so that we're consistent across platforms, but compatible with IE
in quirks mode. That means we'd still need this fix.
I don't understand why this patch would still be necessary once bug 91592 is
fixed. The browser XUL isn't in quirks mode, so I wouldn't expect a quirks-mode
exception to affect browser menus.
(In reply to comment #4)
> I don't understand why this patch would still be necessary once bug 91592 is
> fixed. The browser XUL isn't in quirks mode, so I wouldn't expect a quirks-mode
> exception to affect browser menus.

Because the keystroke may orginate when the content area has focus. Hmm, that
sounds messy. We better do the same thing no matter what has focus. Back to the
drawing board.
Attachment #169403 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #169403 - Flags: review?(bryner)
Brian and I were discussing this, and we don't think that modifier keys should
ever repeat.
I experience this bug, but in a quite different situation, and it might be
useful to describe it. So:

1. go to www.getfirefox.com (or some site which has large files to download)
2. alt click on the download link to start download (in this case alt is a
shortcut to avoid dialogue)
3. press ctrl+j once (it should call the download manager, but you get beep instead)
4. press ctrl+j again (now download manager appears)
QA Contact: bugzilla → menus
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

I am experiencing a variation on the original bug description.

Steps to reproduce:
1. Start holding Alt.
2. Click with either mouse button (a link or a blank spot on a page).
3. Release Alt.

Result: Menubar gets focus.

It does not seem to matter if you hold alt after clicking or before.  As far as
I can tell the, important piece is whether the total duration of holding alt
(before, during, and after click), has crossed some arbitrary threshold.

Does anyone know if the attached patch or any of the patches referenced in
previous comments were used in the official win32 firefox 1.5b1 build?

TIA,
Jeremy
(In reply to comment #8)
> Does anyone know if the attached patch or any of the patches referenced in
> previous comments were used in the official win32 firefox 1.5b1 build?

I still see this in FF 1.5 RC2.
I would like to comment that this is still present in Firefox 2 nightly builds, and you don't even need to wait 2 seconds for it to happen.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060615 BonEcho/2.0a3 ID:2006061503
Version: 1.0 Branch → Trunk
Attachment #169403 - Attachment is obsolete: true
Mass un-assigning bugs assigned to Aaron.
Assignee: aaronleventhal → nobody
This is a mass change. Every comment has "assigned-to-new" in it.

I didn't look through the bugs, so I'm sorry if I change a bug which shouldn't be changed. But I guess these bugs are just bugs that were once assigned and people forgot to change the Status back when unassigning.
Status: ASSIGNED → NEW
Attached patch Patch v1.0Splinter Review
This bug is more shown up than earlier version because the menubar is autohide in default settings.

If DOM3's repeat property will be implemented, this bug will be fixed simpler. But I think that we should fix this bug before final. When I want to select text from a hyperlink, I press Alt key and start to drag. But when I release Alt key after selection, menubar will be shown unexpectedly.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Attachment #490858 - Flags: review?(enndeakin)
Component: Menus → XUL
Product: Firefox → Core
QA Contact: menus → xptoolkit.widgets
Why do you need two separate fields? It seems that you could just clear mAccessKeyDown when the mouse was pressed.
(In reply to comment #14)
> Why do you need two separate fields? It seems that you could just clear
> mAccessKeyDown when the mouse was pressed.

If I clear mAccessKeyDown, the keydown handler cannot know whether the alt keydown event is for repeated or not. Therefore, current build always recovers the mAccessKeyDown *after* mousedown event handler cleared it.
(In reply to comment #15)
> If I clear mAccessKeyDown, the keydown handler cannot know whether the alt
> keydown event is for repeated or not.

The alt key doesn't repeat when help down.

Or is there some special situation in which it does?
(In reply to comment #16)
> (In reply to comment #15)
> > If I clear mAccessKeyDown, the keydown handler cannot know whether the alt
> > keydown event is for repeated or not.
> 
> The alt key doesn't repeat when help down.
> 
> Or is there some special situation in which it does?

No, alt key is repeated on Windows. I confirmed Linux doesn't repeat. I'm not sure for Mac because I cannot test now.

I think that we can prevent the auto repeated keydown events on Windows because the native key events of Windows have a flag which indicates whether the key event is repeated. However, I'm not sure for other platforms such as Android.

DOM3 event spec said that whether the repeated event comes depends on the system configuration. So, the current Windows' behavior is not invalid.
http://dev.w3.org/2006/webapi/DOM-Level-3-Events/html/DOM3-Events.html#events-KeyboardEvent-repeat
I told to coworkers and they confirmed that Mac doesn't repeat keydown events by modifier keys. So, only Windows repeats modifier keys in tier-1 platforms.
(In reply to comment #17)
> No, alt key is repeated on Windows.

That isn't the behaviour I see. I see no repeating for the alt key on windows.

But I'll assume this might be some platform or keyboard type variation. I'll look at the patch again.
Enn: Would you continue to review it?
Comment on attachment 490858 [details] [diff] [review]
Patch v1.0

It would be good to add a test for this. It can be added to test_menubar.xul
Attachment #490858 - Flags: review?(enndeakin) → review+
(In reply to comment #21)
> Comment on attachment 490858 [details] [diff] [review]
> Patch v1.0
> 
> It would be good to add a test for this. It can be added to test_menubar.xul

Hmm.... I cannot write the test for finding new regression because test_menubar.xul doesn't do correct test.

E.g., DOMMenuBarActive is fired asynchronously. Therefore, my test passed on current trunk too. But the menubar will be active after the test :-(
Attached patch new testcaseSplinter Review
Attachment #493335 - Flags: review?(enndeakin)
In other words, there might be some new regressions which are tested without "events".
Comment on attachment 490858 [details] [diff] [review]
Patch v1.0

The menubar is auto-hide mode in default setting after Fx4. So, Alt+selecting by mouse dragging shouldn't show menu unexpectedly.
Attachment #490858 - Flags: approval2.0?
Attachment #493335 - Flags: review?(enndeakin) → review+
Attachment #490858 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/70cb20ed0813
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Depends on: 616797
You need to log in before you can comment on or make changes to this bug.