Closed Bug 656011 Opened 10 years ago Closed 8 years ago

Saved passwords box does not provide a way to copy username

Categories

(Toolkit :: Password Manager, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: sm-et, Assigned: sankha)

References

Details

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

Attachments

(1 file, 1 obsolete file)

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.
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
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
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
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]
Can you please assign me the bug? I have started working on it.
Assignee: nobody → sankha93
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+
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
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
https://hg.mozilla.org/mozilla-central/rev/87f322ca53e4
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Thanks Sankha for your patch!  Would you mind filing a follow-up bug to add an access key for this menu item as well?
(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?
(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"?  :-)
Blocks: 771215
You need to log in before you can comment on or make changes to this bug.