400893 - Escape should close panels

Status: RESOLVED FIXED

(Firefox :: General, defect, P2)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

Build ID: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a9pre) Gecko/2007102304 Minefield/3.0a9pre

Summary: Need to make a keybinding for "esc" (escape) to close the Larry popup window

For accessibility (sec 508) and usability reasons, we need a keybinding for "esc" (escape) such that it closes the Larry popup dialog.

See "Escape" on http://www.labware.com/LWWeb.nsf/lp/en041101

Steps to Reproduce:

1. Load http://bugzilla.mozilla.org
2. Click on the "bugzilla.mozilla.org" area just to the left of the full "http://bugzilla.mozilla.org" URL
3. Try to close the Larry dialog by pressing "esc" (escape)

Expected Results:

Larry closes upon "esc" (escape)

Actual Results:

Nothing happens

Comment 1

[Neil Deakin]

This applies to all panels, identity popup, add bookmark, popup in download manager, etc. The latter is bug 393188.

Comment 2

[Neil Deakin]

Attachment: check escape for panels

This patch adds keyboard listeners for menus and panels, not just panels as before. Within the keypress handler, check for the escape key and close the panel. Ignore other keys which only apply to menus.

Also make the test use the escape key to close the menu to test this.

Comment 3

[Neil Deakin]

(In reply to comment #2)
> Created an attachment (id=286040) [details]
> check escape for panels
> 
> This patch adds keyboard listeners for menus and panels, not just panels as
> before.

Of course, I meant to say 'not just menus as before'

Comment 4

[[e-mail]]

As this stands, won't this break autocomplete which needs to see the escape key even when the panel is open, so that it can cancel any pending searches etc.

Comment 5

[[e-mail]]

Comment on attachment 286040 [details] [diff] [review]
check escape for panels

As it stands this patch definitely breaks autocomplete.

Comment 6

[Neil Deakin]

Attachment: move ignorekeys check to be done for menus and panels

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 286287 [details] [diff] [review]
move ignorekeys check to be done for menus and panels

a=endgame drivers for after M9 freeze

Comment 8

[Reed Loden [:reed]]

Enn's back, so he can check this in himself. :)

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b2pre) Gecko/2007111304 Minefield/3.0b2pre

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b2pre) Gecko/2007111312 Minefield/3.0b2pre

and

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007111304 Minefield/3.0b2pre

(Tested the Bookmark This Page popup as well as the originally filed issue.)

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

probably, this bug breaks IME handling on Seamonkey. What means |ignorekeys="true"| in autocomplete.xml of xpfe? When this attribute is true, IME cannot be used until the popup is closed. See nsXULPopupManager::UpdateKeyboardListeners.

Comment 11

[Neil Deakin]

(In reply to comment #10) 
> What means |ignorekeys="true"| in autocomplete.xml of xpfe? 

It means that the autocomplete widget doesn't do the built-in escape key handling, and handles it manually. The toolkit one has the same thing, although its set in the constructor for some reason.

Does the presence of the popup interfere with the ime handling?

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #11)
> Does the presence of the popup interfere with the ime handling?

The most popups don't need to kill the IME handling. However, menus have a mnemonic for key access. For handling them, IME handling is killed at opening a menu. I think that the semantic of nsXULPopupManager::UpdateKeyboardListeners() is changed by this bug (or refactoring the XUL popups). I'll file a bug and attach a patch which is changing the nsXULPopupManager::UpdateKeyboardListeners() only calling |nsContentUtils::NotifyInstalledMenuKeyboardListener(PR_TRUE)| at opening the menus.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b7e81a12ee4a
escape should close panels, r+sr=neil,a=beltzner