Display username and password as separate fields in the password doorhanger

VERIFIED FIXED in Firefox 39

Status

()

Toolkit
Password Manager
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: Paolo, Assigned: Paolo)

Tracking

(Blocks: 1 bug, {user-doc-needed})

Trunk
mozilla39
user-doc-needed
Points:
2
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(firefox39 verified)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

3 years ago
To clarify the password doorhanger, separate read-only username and password fields can be used as a first step towards the ability to edit them.
(Assignee)

Updated

3 years ago
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Iteration: --- → 39.2 - 23 Mar
Points: --- → 2
Flags: qe-verify+
Flags: firefox-backlog+
(Assignee)

Comment 1

3 years ago
Created attachment 8578304 [details] [diff] [review]
Work in progress

I should work on removing some of the duplication, but in the meantime there is still material for feedback.
Attachment #8578304 - Flags: feedback?(MattN+bmo)
Comment on attachment 8578304 [details] [diff] [review]
Work in progress

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

Awesome!

::: browser/base/content/popup-notifications.inc
@@ +61,5 @@
>      </popupnotification>
>  
> +    <popupnotification id="password-notification" hidden="true">
> +      <popupnotificationcontent orient="vertical">
> +        <textbox id="password-notification-username" flex="1"

Reminder to remove @flex

::: toolkit/components/passwordmgr/nsLoginManagerPrompter.js
@@ -1023,5 @@
> -                                    "updatePasswordMsg",
> -                                    [displayUser]);
> -    } else {
> -      notificationText  = this._getLocalizedString(
> -                                    "updatePasswordMsgNoUser");

Paolo tells me these strings are still used for old notifications.

@@ +1073,5 @@
> +                 .value = aOldLogin.username;
> +        chromeDoc.getElementById("password-notification-password")
> +                 .value = aNewPassword;
> +
> +        return false;

Remove this r.v. as we discussed in case it gets used for something later

::: toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
@@ +13,5 @@
>  # String is the login's hostname.
>  rememberPasswordMsgNoUsername = Would you like to remember the password on %S?
> +# LOCALIZATION NOTE (usernamePlaceholder):
> +# This is displayed in place of the username when it is missing.
> +usernamePlaceholder=No username

This doesn't match the UX spec so you should probably discuss this with Ryan. Also, I think the ID should include "No" since we will want usernamePlaceholder to be the new value after it's editable. We may want to avoid churn on the string for l10n.
Attachment #8578304 - Flags: feedback?(MattN+bmo) → feedback+
(Assignee)

Comment 3

3 years ago
Created attachment 8578735 [details]
Screenshot

Talked with Ryan and we concluded the "No username" string is better in general even for the final version. Here is a screenshot of how this looks on OS X.
(Assignee)

Updated

3 years ago
Blocks: 1144856
(Assignee)

Updated

3 years ago
Depends on: 1144869
(Assignee)

Comment 4

3 years ago
Created attachment 8579612 [details] [diff] [review]
The patch

The final strings will be different, so I reused the current ones for now.
Attachment #8578304 - Attachment is obsolete: true
Attachment #8579612 - Flags: review?(MattN+bmo)
(Assignee)

Updated

3 years ago
Keywords: user-doc-needed
Attachment #8579612 - Flags: review?(MattN+bmo) → review+
(Assignee)

Comment 5

3 years ago
Created attachment 8580403 [details] [diff] [review]
Updated patch

Fixed test cases that checked the displayed messages.
Attachment #8579612 - Attachment is obsolete: true
(Assignee)

Comment 6

3 years ago
Created attachment 8580406 [details] [diff] [review]
Updated patch

Fixed leftover debugging code and pushed to try:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1191b25feba3
Attachment #8580403 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/aa2b8bc620db
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
QA Contact: kjozwiak
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 # 1145913. 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
status-firefox39: fixed → verified
You need to log in before you can comment on or make changes to this bug.