Closed
Bug 1145913
Opened 9 years ago
Closed 9 years ago
Make the username in the password notification editable
Categories
(Toolkit :: Password Manager, defect)
Toolkit
Password Manager
Tracking
()
Tracking | Status | |
---|---|---|
firefox39 | --- | verified |
People
(Reporter: Paolo, Assigned: Paolo)
References
Details
Attachments
(1 file, 1 obsolete file)
14.94 KB,
patch
|
MattN
:
review+
|
Details | Diff | Splinter Review |
This implements the first part of the editability on capture.
Assignee | ||
Comment 1•9 years ago
|
||
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Attachment #8581009 -
Flags: feedback?(MattN+bmo)
Assignee | ||
Updated•9 years ago
|
Iteration: --- → 39.2 - 23 Mar
Points: --- → 5
Assignee | ||
Updated•9 years ago
|
Flags: qe-verify+
Flags: firefox-backlog+
Assignee | ||
Comment 2•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff9562b36ca5
Assignee | ||
Comment 3•9 years ago
|
||
Now with tests.
Attachment #8581009 -
Attachment is obsolete: true
Attachment #8581009 -
Flags: feedback?(MattN+bmo)
Attachment #8581932 -
Flags: review?(MattN+bmo)
Assignee | ||
Comment 4•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3aa7860b278e
Comment 5•9 years ago
|
||
Comment on attachment 8581932 [details] [diff] [review] The patch Review of attachment 8581932 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/passwordmgr/nsLoginManagerPrompter.js @@ +838,5 @@ > + currentNotification.mainAction.accessKey = accessKey; > + > + // Update the labels in real time if the notification is displayed. > + let element = [...currentNotification.owner.panel.childNodes] > + .find(n => n.notification == currentNotification); We should really expose this as an API on PopupNotifications at some point since enough code reaches into it like this. @@ +843,5 @@ > + if (element) { > + element.setAttribute("buttonlabel", label); > + element.setAttribute("buttonaccesskey", accessKey); > + } > + } Nit: missing semicolon after all of the helper functions.
Attachment #8581932 -
Flags: review?(MattN+bmo) → review+
Assignee | ||
Comment 6•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/56ce9fdb62e3
Backed out for mochitest-5 failures: https://hg.mozilla.org/integration/fx-team/rev/c12899fa2f8a https://treeherder.mozilla.org/logviewer.html#?job_id=2373378&repo=fx-team
Flags: needinfo?(paolo.mozmail)
Updated•9 years ago
|
Iteration: 39.2 - 23 Mar → 39.3 - 30 Mar
Assignee | ||
Comment 8•9 years ago
|
||
For some reason the last fix from bug 1145789 was lost. Relanded with the fix: https://hg.mozilla.org/integration/fx-team/rev/4944dc94d987
Flags: needinfo?(paolo.mozmail)
https://hg.mozilla.org/mozilla-central/rev/4944dc94d987
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Updated•9 years ago
|
QA Contact: kjozwiak
Comment 10•9 years ago
|
||
Went through all the test cases listed below in the etherpad without any issues. I've also included the OS's and websites that were used: * https://etherpad.mozilla.org/Bug1143903-Bug1145913 As per my conversation with Paolo, this was tested in conjunction with bug # 1143903. The main scope was to concentrate on editing the username field in the doorhanger, especially on "change password" pages that don't supply a username. If someone sees a missing case in the etherpad, please let know and I'll add it!
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 11•9 years ago
|
||
(In reply to Kamil Jozwiak [:kjozwiak] from comment #10) > If someone sees a missing case in the etherpad, please let know and I'll add it! Hi Kamil, sorry for the delay. I've taken a look at the Etherpad and that's a pretty impressive set of tests! It will probably be worth ensuring that at least some of them are defined as recurring tests in our manual test suites. I have only two notes specific to this bug: * Ensure that you can use the same username for multiple websites without any collision issues ** Ensure that changing the password of a username that's being used multiple times doesn't cause any collision issues It might be better to expand "collision issues" into the exact steps taken to verify no collisions occurred. * Ensure that the username field under the password doorhanger is editable Did you do any specific tests for possible "expected" collisions as a result of changing the username in the dialog? In particular we implemented different button labels based on whether clicking the button will overwrite an existing login or not.
Comment 12•9 years ago
|
||
Thanks for the feedback Paolo, much appreciated! > I have only two notes specific to this bug: > > * Ensure that you can use the same username for multiple websites without > any collision issues > ** Ensure that changing the password of a username that's being used > multiple times doesn't cause any collision issues > > It might be better to expand "collision issues" into the exact steps taken > to verify no collisions occurred. Agreed! Added several test cases into the wiki that will cover checks for possible collisions. > * Ensure that the username field under the password doorhanger is editable > > Did you do any specific tests for possible "expected" collisions as a result > of changing the username in the dialog? In particular we implemented > different button labels based on whether clicking the button will overwrite > an existing login or not. Do you mean something similar to the following cases? - login into SiteA and save the credentials - login into SiteB with the same username as SiteA - delete the current username from the doorhanger and re-enter the same username once again - you should still have "Remember Password" rather than "Update Password" because that user doesn't exists under SiteB - make sure SiteA's credentials are not being affected - login into SiteA and save the credentials - login into SiteB and save the credentials - login into SiteA with a different username - in the doorhanger, change the current username to the one you used previously, "Remember Password" should change to "Update Password" - add a completely different username under the doorhanger that hasn't been saved on SiteA and "Update" should be change back to "Remember Password" - make sure SiteB is never affected with the changes being made in SiteA
Flags: needinfo?(paolo.mozmail)
Updated•9 years ago
|
Blocks: 2015-login-capture-UX
Updated•9 years ago
|
No longer blocks: passwords-2015-UX
Assignee | ||
Comment 13•9 years ago
|
||
(In reply to Kamil Jozwiak [:kjozwiak] from comment #12) > Do you mean something similar to the following cases? Yes, they look good to me!
Flags: needinfo?(paolo.mozmail)
You need to log in
before you can comment on or make changes to this bug.
Description
•