212641 - Alt+Down in URL bar leads to historty popup that won't go away

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Aaron Leventhal]

Discovered this while working on bug 192577, although it's mostly unrelated.

Steps to repro on Windows:
Ctrl+L
Alt+Down arrow
Click outside popup, even in another app

Results:
Popup stays open and on top, even if you clicked in another app that should have
hidden Mozilla.

Comment 1

[Aaron Leventhal]

Attachment: Don't treat Alt+down or alt+nonprintablekey as accesskey

Comment 2

[beanladen]

WFM on Linux 1.5a i686 25-Jun-2003.
I like these keys, so maybe the responsible bug should be fixed instead of 
killing them totally.

Comment 3

[Aaron Leventhal]

> WFM on Linux 1.5a i686 25-Jun-2003.
Then this must be a Windows bug only.

> I like these keys, so maybe the responsible bug should be fixed instead of 
> killing them totally.

The patch doesn't kill Alt+Down arrow, it just doesn't try to treat it as an
underlined accesskey, which was causing the menu dismissal listener to not get
attached correctly. The fix makes everything work as it should.

Comment 4

[[e-mail]]

Comment on attachment 127722 [details] [diff] [review]
Don't treat Alt+down or alt+nonprintablekey as accesskey

This looks like a good thing to do, but I can't test it :-(

Comment 5

[Aaron Leventhal]

Comment on attachment 127722 [details] [diff] [review]
Don't treat Alt+down or alt+nonprintablekey as accesskey

Akkana, do you have time to review these changes to your code? Thanks.

Comment 6

[Akkana Peck]

(Not originally my code; that was just code I rearranged while adding some
configurability.  But in any case this should probably be reviewed/tested by
someone with a windows build if this is a windows-only issue.)  

Renaming theChar to keyCode is fine by me, but are we confident that "no
charcode" always correlates with "shouldn't be used as access key"?  If so, it
might be good to put a comment in the code saying so; just checking for
non-nullness of charcode was confusing to me when I was reading the code.

Comment 7

[Brian Ryner (not reading)]

Comment on attachment 127722 [details] [diff] [review]
Don't treat Alt+down or alt+nonprintablekey as accesskey

This makes sense to me, assuming that no charcode always means that you don't
have a printable character (I think it does).

Comment 8

[Aaron Leventhal]

Comment on attachment 127722 [details] [diff] [review]
Don't treat Alt+down or alt+nonprintablekey as accesskey

Will add comment to code, saying that charCode of 0 indicates not printable.

Comment 9

[timeless]

this sounds right from memory.

Comment 10

[Asa Dotzler [:asa]]

It looks like this landed on 7/25.  Can this bug be resolved?

Comment 11

[Ryuichi KUBUKI]

Can someone check the regression caused by the patch as bug 215787? Kind of
little feedback there.

Comment 12

[timeless]

mozilla/layout/xul/base/src/nsMenuBarListener.cpp 1.66