431901 - Update preference dialogs to treat backspace and delete equivalently on Mac OS X

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect)

Description

[Minh Nguyễn]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b5) Gecko/2008032619 Firefox/3.0b5
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b5) Gecko/2008032619 Firefox/3.0b5

In Firefox's Cookies dialog, the selected cookie can be deleted by pressing the Remove Cookie button or the Delete Forward key (Del on platforms other than the Mac). On Mac laptops, which don't have a dedicated DelFwd key, Fn+Delete is used -- not a very discoverable shortcut. So in addition to DelFwd, the dialog should allow the Delete (Backspace) key to remove the cookie. This behavior would be consistent with other Mac applications, such as Safari (Delete to delete a cookie), Camino (same), and Cyberduck (Delete to delete a file).

Reproducible: Always

Steps to Reproduce:
1. In Firefox (Mac), open the Preferences dialog.
2. Go to the Privacy tab, and click the Show Cookies button.
3. Select a site or individual cookie, and press Delete (Backspace).
4. Now press Delete Forward or Fn+Delete.
Actual Results:  
Pressing Delete doesn't remove the cookie, whereas pressing Delete Forward (or Fn+Delete) does.

Expected Results:  
Both shortcuts should remove the cookie.

This bug also affects the Popups Exceptions, Images Exceptions, Cookies Exceptions, Saved Passwords, and Addons Exceptions dialogs.

Comment 1

[Tyler Downer [He/Him]]

This bug was reported using Firefox 3.0 or older, which is no longer supported. The bug has also not been changed in over 500 days and is still in UNCO.
Reporter, please retest this bug in Firefox 3.6.10 or later using a fresh profile, http://support.mozilla.com/en-US/kb/managing+profiles. If you still see this problem, please update the bug. If you no longer see the bug, please set the resolution to RESOLVED, WORKSFORME.

This is a mass search of unconfirmed bugs that have no activity on them, so if you feel a bug was marked in error, just remove the CLOSEME comment in the whiteboard within the next month.

Comment 2

[Minh Nguyễn]

Backspace still doesn't work in trunk.

Comment 3

[Michael Pruett]

Attachment: Patch

The problem is that the cookies dialog listens only for key presses with key code 46 (delete) and not key code 8 (backspace).

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 8414949 [details] [diff] [review]
Patch

Thanks for the patch Michael!

While you're at it, would you mind changing these to using the named constants instead of hardcoded numbers? Like this:

if (aEvent.keyCode == KeyEvent.DOM_VK_BACK_SPACE ||
    aEvent.keyCode == KeyEvent.DOM_VK_DELETE)

r=me with that change.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Hmm, though perhaps this change should only be made on Mac?

I'm not sure whether there's a consistency issue here with our behavior elsewhere.

Comment 6

[Michael Pruett]

Attachment: Patch

I don't see a reason to make this change only for Mac OS. Many modern laptop keyboards have inconveniently located delete keys. And in contexts unrelated to text editing, there is no meaningful distinction between the delete and backspace keys.

Consistency would indeed require making this change in all preference dialogs exhibiting the same behavior. I've updated the patch accordingly.

Comment 7

[Matthew N. [:MattN]]

Comment on attachment 8416279 [details] [diff] [review]
Patch

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

::: browser/components/preferences/cookies.js
@@ +717,5 @@
>    },
>  
>    onCookieKeyPress: function (aEvent) {
> +    if (aEvent.keyCode == KeyEvent.DOM_VK_BACK_SPACE ||
> +        aEvent.keyCode == KeyEvent.DOM_VK_DELETE)

Since there is a filter text field in some of these dialogs I don't think we should switch to non-standard behaviour on Windows (I don't know what's normal on Linux). Someone may hit backspace thinking the focus was in the field and end up deleting data. I don't know anywhere else in product that we delete with backspace (and no modifiers) either. Can you put the KeyEvent.DOM_VK_BACK_SPACE addition inside an #ifdef XP_MACOSX like so:
  if (aEvent.keyCode == KeyEvent.DOM_VK_DELETE
#ifdef XP_MACOSX
      || aEvent.keyCode == KeyEvent.DOM_VK_BACK_SPACE
#endif
     )

Comment 8

[Michael Pruett]

Attachment: Patch

I've updated the patch to treat backspace as equivalent to delete in preference dialogs only on Mac OS X.

https://tbpl.mozilla.org/?tree=Try&rev=607324c8ee9f

Comment 9

[Matthew N. [:MattN]]

Comment on attachment 8421789 [details] [diff] [review]
Patch

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

Please ensure you have manually tested profileSelection.js since it's not covered by automation AFAIK.

Thanks Michael!

Comment 10

[Matthew N. [:MattN]]

The try server results in comment 8 look good so go ahead and add the checkin-needed keyword once you have completed manual testing. I'm mostly concerned about pre-processor issues.

Comment 11

[Michael Pruett]

I've validated these changes manually on Mac OS X and Linux.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/19c27603fded

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/19c27603fded

Comment 14

[Camelia Badau [:cbadau], Desktop QA]

Verified on Windows 7 64bit, Ubuntu 13.10 32bit using Firefox 32 Beta 8 (buildID: 20140818191513) -- the backspace is not equivalent to delete in preference dialogs

Verified on Mac OSX 10.8.5 using Firefox 32 Beta 8 (buildID: 20140818191513) -- the backspace is equivalent to delete in preference dialogs