Saved passwords box does not provide a way to copy username

RESOLVED FIXED in mozilla16

Status

()

Toolkit
Password Manager
--
enhancement
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: Mike Semkey, Assigned: sankha)

Tracking

Trunk
mozilla16
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mentor=MattN][lang=js][lang=xul])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
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

7 years ago
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
OS: FreeBSD → All
Hardware: x86_64 → All
Version: unspecified → 3.6 Branch

Comment 2

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

Considering Comment 1, setting resolution to New.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

6 years ago
Severity: normal → enhancement
Summary: Saved passwords box does not provide a way to copy username/password → Saved passwords box does not provide a way to copy username
Version: 3.6 Branch → Trunk

Updated

6 years ago
Depends on: 754398
Duplicate of this bug: 768280
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
Component: Preferences → Password Manager
Product: Firefox → Toolkit
QA Contact: preferences → password.manager
Whiteboard: [mentor=MattN][lang=js][lang=xul]
(Assignee)

Comment 5

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

Updated

6 years ago
Assignee: nobody → sankha93
(Assignee)

Comment 6

6 years ago
Created attachment 636835 [details] [diff] [review]
password manager allows to copy username
Attachment #636835 - Flags: review?(mnoorenberghe+bmo)
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", ...).
Attachment #636835 - Flags: review?(mnoorenberghe+bmo)
Attachment #636835 - Flags: review?(dolske)
Attachment #636835 - Flags: review+
(Assignee)

Comment 8

6 years ago
Created attachment 639165 [details] [diff] [review]
Added the suggested changes to the old patch
Attachment #636835 - Attachment is obsolete: true
Attachment #636835 - Flags: review?(dolske)
Attachment #639165 - Flags: review?(mnoorenberghe+bmo)
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?
Attachment #639165 - Flags: review?(mnoorenberghe+bmo)
Attachment #639165 - Flags: review?(dolske)
Attachment #639165 - Flags: review+
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.
Attachment #639165 - Flags: review?(dolske) → feedback+
Pushed to mozilla-inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/87f322ca53e4
Status: NEW → ASSIGNED
Flags: in-testsuite+
Target Milestone: --- → mozilla16

Comment 12

6 years ago
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/mozilla@noorenberghe.ca-966329eade22

Comment 13

6 years ago
https://hg.mozilla.org/mozilla-central/rev/87f322ca53e4
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Comment 14

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

Comment 15

6 years ago
(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

6 years ago
(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"?  :-)
(Assignee)

Updated

5 years ago
Blocks: 771215
You need to log in before you can comment on or make changes to this bug.