407498 - "Save password in Keychain" default focused button changed in 1.6a1. Can I at least have a key equivalent?

Status: RESOLVED FIXED

(Camino Graveyard :: OS Integration, defect)

Description

[Mark Mentovai]

In Camino 1.5.x, the "Save this password in your Keychain?" dialog's default focused button is "Don’t Save".  This means that for users with Full Keyboard Access turned on, the default action when pressing the space bar is to trigger the "Don’t Save" action.

In Camino 1.6a1, the default action has moved to "Never Save for this Website".  It looks like this happened in bug 369245, when the sheet moved from Keychain.nib (which had KeychainSavePrompt's initialFirstResponder set to the "Don’t Save" button) to being generated directly by code in KeychainService.mm.

I recall that there was some discussion about what to do with the initialFirstResponder in 3-button sheets, and I recall that the outcome of that discussion pissed me off slightly.  If this change was intentional, then that's cool, since I'm only pissed off slightly.  I'll be placated simply by assigning a key equivalent to the "Don’t Save" button.

If this change wasn't intentional, then, um, it's a regression.

Comment 1

[Stuart Morgan]

It wasn't intentional, and Never Save is certainly not ideal.

Comment 2

[Mark Mentovai]

Attachment: Fix initial first responder and add back key equivalents

I pulled the key equivalents out of the old nib.  Command-N for Never Save isn't localizable with this change.  Do we need it to be?  Did any translation use a different key equivalent when this was nib-based?  Do we care?

While I was in there, I added the Esc equivalent back to the other dialog that used to come from the nib and now is built up as an NSAlert panel in code.

The last thing I need to do is whack all of the things that got added to the Keychain Deny List when my trigger-happy thumb hit the space bar.  There's (doh!) no way to do that from the UI.

Comment 3

[Stuart Morgan]

Comment on attachment 292240 [details] [diff] [review]
Fix initial first responder and add back key equivalents

(In reply to comment #2)
> There's (doh!) no way to do that from the UI.

Yeah, that's bug 354975. At least it's a plist now instead of a binary blob of serialized data ;)

Comment 4

[Mark Mentovai]

Checked in on the trunk.

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Marcello, can you weigh in on the first part of mento's comment 2?  My recollection is that only keystrokes that became very hard to use on keyboards of other languages got changed....

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #4)
> Checked in on the trunk.

Er, what about the branch? ;)

Comment 7

[Mark Mentovai]

I wasn't sure if we changed the chicken rules post-alpha.

Comment 8

[Mark Mentovai]

Checked in on MOZILLA_1_8_BRANCH before 1.8b1.  Will monitor this bug for feedback re comments 2 and 5.

Comment 9

[Marcello Testi]

(In reply to comment #2)
> I pulled the key equivalents out of the old nib.  Command-N for Never Save
> isn't localizable with this change.  Do we need it to be?  Did any translation
> use a different key equivalent when this was nib-based?  Do we care?

Smokey is right: l10n of keyboard equivalents has been discussed on Caminol10n only when issues arose with keyboard layout incompatibilities.
OTOH, I just checked Camino 1.5.4Int (Italian) and found out that cmd-n was the ONLY keyboard equivalent working with the "save password" sheet. I expected cmd-d for "Don't save" and cmd-s for "Save", since I think this is the way it works for every localized app, at least with Italian.
All in all, my personal experience tells me that keyboard equivalents are not localized (I can't say wether they're localizable) for common tasks. For example, in Italian we have cmd-h for "Hide" ("Nascondi") and cmd-m for "Minimize" ("Riduci").

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #9)
> OTOH, I just checked Camino 1.5.4Int (Italian) and found out that cmd-n was the
> ONLY keyboard equivalent working with the "save password" sheet. I expected
> cmd-d for "Don't save" and cmd-s for "Save", since I think this is the way it
> works for every localized app, at least with Italian.

Bug 330299.