656011 - Saved passwords box does not provide a way to copy username

Status: RESOLVED FIXED

(Toolkit :: Password Manager, enhancement)

Description

[Mike Semkey]

User-Agent:       Mozilla/5.0 (X11; U; FreeBSD amd64; en-US; rv:1.9.2.16) Gecko/20110411 Firefox/3.6.16
Build Identifier: Mozilla/5.0 (X11; U; FreeBSD amd64; en-US; rv:1.9.2.16) Gecko/20110411 Firefox/3.6.16

Looking at the saved passwords box there does not appear to be any way to copy either username or password to clipboard.

This is useful when the same password needs to be entered on a different site, or on a different computer.

Since passwords that must be remembered are frequently resembling gibberish typing them out by hand is rather hard.

If the password needs to be entered on a different site (still in firefox), there is the added complication of having to keep saved passwords open at the same time time as this different site. Frequently this necessitates opening a new window since the preferences are modal.

Usernames are usually easier to type than passwords but since sometimes usernames are email addresses they can be quite long to type, and therefore would also benefit from being copyable.

Reproducible: Always

Steps to Reproduce:
1. Go to Preferences -> Security -> Saved passwords
2. Observe saved passwords box appearing

Actual Results:  
No way to copy username/password.

Expected Results:  
Buttons to copy username/password, context menu with options to copy username/password, etc.

Comment 1

[Thomas Ahlblom]

Reproduced (no way to copy username/password).
Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2.17) Gecko/20110420 Firefox/3.6.17

Password, but not username, can be copied.
Mozilla/5.0 (X11; Linux x86_64; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Mozilla/5.0 (X11; Linux x86_64; rv:6.0a1) Gecko/20110514 Firefox/6.0a1
Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110514 Firefox/6.0a

Comment 2

[George Carstoiu]

Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110503 Firefox/6.0a1

Considering Comment 1, setting resolution to New.

Comment 4

[Matthew N. [:MattN]]

This seems like a reasonable request to me.  Here are some resources for someone who wants to implement this:

XUL: 
https://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/content/passwordManager.xul#32

UpdateCopyPassword: 
https://mxr.mozilla.org/mozilla-central/search?string=UpdateCopyPassword

Test for copy password: 
https://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/test/browser/browser_passwordmgrcopypwd.js

Where to add the localizable string: 
https://mxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/passwordmgr/passwordManager.dtd#29

Comment 5

[Sankha Narayan Guria [:sankha]]

Can you please assign me the bug? I have started working on it.

Comment 6

[Sankha Narayan Guria [:sankha]]

Attachment: password manager allows to copy username

Comment 7

[Matthew N. [:MattN]]

Comment on attachment 636835 [details] [diff] [review]
password manager allows to copy username

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

Looks good! Just some minor renaming and re-ordering to do. Sorry for the delay.  I'll get dolske to double-check this since he's an owner of the code.

::: toolkit/components/passwordmgr/content/passwordManager.js
@@ +324,5 @@
>    var password = signonsTreeView.getCellText(row, {id : "passwordCol" });
>    clipboard.copyString(password);
>  }
>  
> +function CopyUser() {

=> "CopyUsername"

@@ +330,5 @@
> +  var clipboard = Components.classes["@mozilla.org/widget/clipboardhelper;1"].
> +                  getService(Components.interfaces.nsIClipboardHelper);
> +  var row = document.getElementById("signonsTree").currentIndex;
> +  var user = signonsTreeView.getCellText(row, {id : "userCol" });
> +  clipboard.copyString(user);

s/var user/var username/

@@ +337,4 @@
>  function UpdateCopyPassword() {
>    var singleSelection = (signonsTreeView.selection.count == 1);
> +  var passwordMenuitem = document.getElementById("context-copypassword");
> +  var userMenuitem = document.getElementById("context-copyuser");

"usernameMenuitem" (below too) and "context-copyusername"

::: toolkit/components/passwordmgr/content/passwordManager.xul
@@ +36,5 @@
>                  accesskey="&copyPasswordCmd.accesskey;"
>                  oncommand="CopyPassword()"/>
> +      <menuitem id="context-copyuser"
> +                label="&copyUserCmd.label;"
> +                oncommand="CopyUser()"/>

Could you put this menuitem before the password one as it seems more natural that way (aligns with login forms and the order of the columns)?

Nit: Let's use "Username" in identifiers instead of "User" to be more clear.

::: toolkit/components/passwordmgr/test/browser/browser_passwordmgrcopypwd.js
@@ +36,5 @@
>      // Test if "Copy Password" works
>      function doTest() {
>          let doc = pwmgrdlg.document;
>          let selection = doc.getElementById("signonsTree").view.selection;
> +        let menuitem = doc.getElementById("context-copyuser");

"context-copyusername"

@@ +43,2 @@
>              selection.selectAll();
>              is(isMenuitemEnabled(), false, "Copy Password should be disabled");

The 3rd argument to ok(..) should be updated to take into account the fact that this function runs for usernames and passwords.  Perhaps just remove "Password" from all 4 of the messages and add |info("Testing Copy Password")| and
|info("Testing Copy Username")| before copyField runs (top of testPassword and before waitForClipboard("ehsan", ...).

Comment 8

[Sankha Narayan Guria [:sankha]]

Attachment: Added the suggested changes to the old patch

Comment 9

[Matthew N. [:MattN]]

Comment on attachment 639165 [details] [diff] [review]
Added the suggested changes to the old patch

Perfect! Dolske can you take a quick look or is my review sufficient?

Comment 10

[Justin Dolske [:Dolske]]

Comment on attachment 639165 [details] [diff] [review]
Added the suggested changes to the old patch

Looks good to me, Matt's r+ is sufficient here.

Comment 11

[Matthew N. [:MattN]]

Pushed to mozilla-inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/87f322ca53e4

Comment 12

[Mozilla RelEng Bot]

Try run for 966329eade22 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=966329eade22
Results (out of 185 total builds):
    success: 168
    warnings: 12
    failure: 5
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-966329eade22

Comment 13

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/87f322ca53e4

Comment 14

[(no longer active)]

Thanks Sankha for your patch!  Would you mind filing a follow-up bug to add an access key for this menu item as well?

Comment 15

[Sankha Narayan Guria [:sankha]]

(In reply to Ehsan Akhgari [:ehsan] from comment #14)
> Thanks Sankha for your patch!  Would you mind filing a follow-up bug to add
> an access key for this menu item as well?

Sure not a problem. I would be interested in working on it also. But what could be a good access key for the "Copy Username" menuitem?

Comment 16

[(no longer active)]

(In reply to comment #15)
> (In reply to Ehsan Akhgari [:ehsan] from comment #14)
> > Thanks Sankha for your patch!  Would you mind filing a follow-up bug to add
> > an access key for this menu item as well?
> 
> Sure not a problem. I would be interested in working on it also. But what could
> be a good access key for the "Copy Username" menuitem?

Not sure, let's say "U"?  :-)