668903 - Pressing escape should close the password manager

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect)

Description

[Nochum Sossonko [:Natch]]

The password manager doesn't close when hitting escape, as does nearly every other dialog/pop-up (e.g. Downloads, Cookies, etc.)

Comment 1

[Nochum Sossonko [:Natch]]

Attachment: Add ESC key handling

Comment 2

[Matthew N. [:MattN]]

Comment on attachment 8402862 [details] [diff] [review]
Add ESC key handling

Review of attachment 8402862 [details] [diff] [review]:
-----------------------------------------------------------------

Hello, thanks for the patch.

I can't get this to work on OS X 10.9 or Windows 7. Were you able to get this to work somewhere?

Also, please make sure that the dialog doesn't close when Escape is pressed in the search field (as that clears the field).

Comment 3

[Nochum Sossonko [:Natch]]

(In reply to Matthew N. [:MattN] from comment #2)
> I can't get this to work on OS X 10.9 or Windows 7. Were you able to get
> this to work somewhere?

Yeah, it's supposed to be "keycode=VK_ESCAPE", I'll correct that and actually test this time.

> Also, please make sure that the dialog doesn't close when Escape is pressed
> in the search field (as that clears the field).

I'll test this as well.

Comment 4

[Nochum Sossonko [:Natch]]

Attachment: Updated to keycode

This fixes both comments :)

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8403488 [details] [diff] [review]
Updated to keycode

Review of attachment 8403488 [details] [diff] [review]:
-----------------------------------------------------------------

Normally I would ask for a test but since this is straightforward enough and I see this behavior tested in other dialogs so I'll let it go ;)

I like this for the consistency with other dialogs as this bothers me too. I do wonder if this will now interfere with user's muscle memory if they're trained they can hit ESC repeatedly to clear the filter without closing it. This patch would make it work like the Applications preferences tab, extension installation whitelist dialog, and the cookie dialog so I think people can get used to that.

Comment 6

[Matthew N. [:MattN]]

Note to pusher: the bug # and reviewer are not in the commit message

Comment 7

[Nochum Sossonko [:Natch]]

Attachment: For Checkin (with bug # and reviewer)

(In reply to Matthew N. [:MattN] from comment #6)
> Note to pusher: the bug # and reviewer are not in the commit message

Fixed. Thanks!

Comment 8

[Matthew N. [:MattN]]

Thank you
https://hg.mozilla.org/integration/fx-team/rev/0df48cf38d63

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0df48cf38d63

Comment 12

[Jan [:Jan\]]

I don't understand why this was reported. It doesn't happen for stable (31) or beta (32).

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0

Comment 13

[Nochum Sossonko [:Natch]]

It was fixed in 31, so you would have to compare to 30 where it was not.