302164 - Yes/No buttons in the "Remember password" dialog should describe their actions, have accesskeys

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect, P2)

Description

[Asaf Romano (gone)]

In the "Remember password" dialog, we're using Yes/No buttons without making
their label a bit more informative, suggesting "Do Not Remember" and "Remember".

Comment 1

[Asaf Romano (gone)]

Attachment: patch

Comment 2

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

The patch would make the dialog look like:

 .-------------------------------------------------------------------------.
 | Password Manager can remember this logon and enter it automatically the |
 | next time you return to this web site.                                  |  
 |                                                                         |
 | Do you want Password Manager to remember this logon?                    |
 |                                                                         |
 |                     [Never for this site]  [Do not rember]  [Remember]  |
 .-------------------------------------------------------------------------.
  
which seems really wordy to me and there's a lot of repetition of the word
"remember". I think that if we change the button labels, we can probably ditch a
lot of the text. Maybe:

 .-------------------------------------------------------------------------.
 | Do you want Firefox to remember this logon and password for next time?  | 
 |                                                                         |
 |                                                                         |
 |                           [Never for this site]  [Not now]  [Remember]  |
 .-------------------------------------------------------------------------.

so:

savePasswordText = Do you want Firefox to remember this logon and password for
next time?
rememberPassword = Remember
doNotRememberPassword = Not now
neverForSite = Never for this site

Comment 3

[Asaf Romano (gone)]

looks good to me.

Comment 4

[Corinne Bordelon]

Not blocking for this - but we'll take the fix if the patch can be made ready in
the next few days.

Comment 5

[Asaf Romano (gone)]

Attachment: patch

Comment 6

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

Attachment: updated patch with new labels

"Do you want Firefox to remember this password?"

  [ Never for this site ] [ Not now ] [ Remember ]

Comment 7

[Asaf Romano (gone)]

Comment on attachment 191615 [details] [diff] [review]
updated patch with new labels

yeek

Comment 8

[Asaf Romano (gone)]

Attachment: patch

Comment 9

[Asaf Romano (gone)]

Checking in toolkit/components/passwordmgr/base/nsPasswordManager.cpp;
/cvsroot/mozilla/toolkit/components/passwordmgr/base/nsPasswordManager.cpp,v 
<--  nsPasswordManager.cpp
new revision: 1.63; previous revision: 1.62
done
Checking in toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties,v
 <--  passwordmgr.properties
new revision: 1.3; previous revision: 1.2
don

Comment 10

[:deanis74]

Reopening.  We lost access keys on the buttons with this change.  Looks like
they are automatically added for Yes/No/Yes to All.

Comment 11

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

While it's reopen, we should probably fix the fact that it looks pretty crappy
on Windows:

 - reorder buttons as shown
 - left-align the "Never for this site"
 - make not-now the default selection

 .-------------------------------------------------------.
 |  .                                                    | 
 | /?\  Do you want Firefox to remember this password?   |
 | ---                                                   |
 |                                                       |
 | [Never for this site]          [Remember]  [Not Now]  |
 .--------------------------------†----------------------.

Comment 12

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

Attachment: w32 screenshot - ew

After a heated discussion in IRC in which I was informed a) how out to lunch I
was, and b) how hard that suggestion would be to implement, I guess that the
ugly w32 dialog can stay for branch. :(

For the record, the problems in w32 are that the new, simpler question means
that the dialog is cramped (see screenshot) which makes it harder to parse at a
glance and results in the buttons bumping up beneath the ? icon which looks
pretty strange.

Suggestion for access keys is: "R"emember, Ne"v"er for this Site, "N"ot Now

Comment 13

[Tracy Walker [:tracy]]

Our language seems to be moving to "Remember" instead of "Save"  

In Options | Passwords, we have Remember Password checkbox.  In the same dialog
we have "View Saved Passwords" button.  Not View Remembered Passwords.

In Options | History we have "Remember visited blah... checkbox.

In Options | Saved Forms (note Save, not Remembered Forms), "Save information I
blah... checkbox and "Clear Saved Form Data Bow" button.

I'm a proponent of consistancy.  The areas I mentioned above are outside of this
bug.  But this seems like a good place to try to move things back to consitancy 

Comment 14

[Simon Bünzli]

Attachment: access keys (r/v/n)

This should fix bug 61877 as well.

Comment 15

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 193911 [details] [diff] [review]
access keys (r/v/n)

r=vladimir with much happiness; my finger memory thanks you

Comment 16

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

Comment on attachment 193911 [details] [diff] [review]
access keys (r/v/n)

Seems pretty low risk to take it, although there may be a late l10n impact.

Comment 17

[Phil Ringnalda (:philor)]

*** Bug 303689 has been marked as a duplicate of this bug. ***

Comment 18

[Asaf Romano (gone)]

Comment on attachment 193911 [details] [diff] [review]
access keys (r/v/n)

We have to change the entity name if we want l10n to be fixed as well

Comment 19

[Asaf Romano (gone)]

Attachment: with that change

Comment 20

[Asaf Romano (gone)]

Comment on attachment 194241 [details] [diff] [review]
with that change

oops, wrong file.

Comment 21

[Asaf Romano (gone)]

Attachment: with that change

Comment 22

[Asaf Romano (gone)]

Please open new bugs on anything else, thanks.

Trunk:
Checking in components/passwordmgr/base/nsPasswordManager.cpp;
/cvsroot/mozilla/toolkit/components/passwordmgr/base/nsPasswordManager.cpp,v 
<--  nsPasswordManager.cpp
new revision: 1.67; previous revision: 1.66
done
Checking in locales/en-US/chrome/passwordmgr/passwordmgr.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties,v
 <--  passwordmgr.properties
new revision: 1.5; previous revision: 1.4
done

1.8 Branch:
Checking in locales/en-US/chrome/passwordmgr/passwordmgr.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties,v
 <--  passwordmgr.properties
new revision: 1.4.2.1; previous revision: 1.4
done
Checking in components/passwordmgr/base/nsPasswordManager.cpp;
/cvsroot/mozilla/toolkit/components/passwordmgr/base/nsPasswordManager.cpp,v 
<--  nsPasswordManager.cpp
new revision: 1.65.2.1; previous revision: 1.65
done