Closed Bug 400893 Opened 13 years ago Closed 3 years ago

Escape should close panels

Categories

(Firefox :: General, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Firefox 3 beta2

People

(Reporter: stephend, Assigned: enndeakin)

References

()

Details

(Keywords: access)

Attachments

(1 file, 1 obsolete file)

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
Flags: blocking-firefox3?
This applies to all panels, identity popup, add bookmark, popup in download manager, etc. The latter is bug 393188.
Assignee: nobody → enndeakin
Summary: Need to make a keybinding for "esc" (escape) to close the Larry popup window → Escape should close panels
Attached patch check escape for panels (obsolete) — Splinter Review
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.
Attachment #286040 - Flags: superreview?(neil)
Attachment #286040 - Flags: review?(neil)
(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'
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 on attachment 286040 [details] [diff] [review]
check escape for panels

As it stands this patch definitely breaks autocomplete.
Attachment #286040 - Flags: superreview?(neil)
Attachment #286040 - Flags: review?(neil)
Attachment #286040 - Flags: review-
Attachment #286040 - Attachment is obsolete: true
Attachment #286287 - Flags: superreview?(neil)
Attachment #286287 - Flags: review?(neil)
Attachment #286287 - Flags: superreview?(neil)
Attachment #286287 - Flags: superreview+
Attachment #286287 - Flags: review?(neil)
Attachment #286287 - Flags: review+
Flags: blocking-firefox3? → blocking-firefox3+
Target Milestone: --- → Firefox 3 M10
Blocks: 393188
Attachment #286287 - Flags: approvalM9?
Comment on attachment 286287 [details] [diff] [review]
move ignorekeys check to be done for menus and panels

a=endgame drivers for after M9 freeze
Attachment #286287 - Flags: approvalM9?
Attachment #286287 - Flags: approvalM9-
Attachment #286287 - Flags: approval1.9+
Keywords: checkin-needed
Priority: -- → P2
Enn's back, so he can check this in himself. :)
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
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.)
Status: RESOLVED → VERIFIED
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.
(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?

(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.
Flags: in-testsuite+
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/b7e81a12ee4a
escape should close panels, r+sr=neil,a=beltzner
Status: VERIFIED → RESOLVED
Closed: 13 years ago3 years ago
You need to log in before you can comment on or make changes to this bug.