Closed Bug 1174815 Opened 5 years ago Closed 5 years ago

Capture doorhanger password field always shows caret at the beginning when clicked.

Categories

(Toolkit :: Password Manager, defect, P1)

defect
Points:
3

Tracking

()

VERIFIED FIXED
mozilla41
Iteration:
41.3 - Jun 29
Tracking Status
firefox38 --- unaffected
firefox38.0.5 --- unaffected
firefox39 --- unaffected
firefox40 --- verified
firefox41 --- verified

People

(Reporter: rittme, Assigned: rittme)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

Bug 1153217 allow editing the password content at the capture doorhanger and bug 1169702 allows showing it. When the field is clicked for editing the caret is always positioned at the beginning of the content and not based on the click position.
Assignee: nobody → bernardo
Status: NEW → ASSIGNED
Bug 1174815 - Caret in capture doorhanger password field is now positioned at the click position offset
Attachment #8622816 - Flags: review?(MattN+bmo)
Comment on attachment 8622816 [details]
MozReview Request: Bug 1174815 - Caret in capture doorhanger password field is now positioned at the click position offset

Bug 1174815 - Caret in capture doorhanger password field is now positioned at the click position offset
Iteration: --- → 41.3 - Jun 29
Points: --- → 3
Flags: qe-verify+
Flags: firefox-backlog+
Comment on attachment 8622816 [details]
MozReview Request: Bug 1174815 - Caret in capture doorhanger password field is now positioned at the click position offset

https://reviewboard.mozilla.org/r/11447/#review9817

::: toolkit/components/passwordmgr/nsLoginManagerPrompter.js:867
(Diff revision 2)
> -      chromeDoc.getElementById("password-notification-password").type = "";
> +      var passwordField = chromeDoc.getElementById("password-notification-password");
> +      var selectionStart = passwordField.selectionStart;
> +      var selectionEnd = passwordField.selectionEnd;

Nit: `let` is the new `var`

::: toolkit/components/passwordmgr/nsLoginManagerPrompter.js:874
(Diff revision 2)
> +      passwordField.removeAttribute("type");

Why did you switch from using the setter to using the attribute? It's generally preferred to use the former.

::: toolkit/components/passwordmgr/nsLoginManagerPrompter.js:868
(Diff revision 2)
> +      var selectionStart = passwordField.selectionStart;
> +      var selectionEnd = passwordField.selectionEnd;

Add a comment above this about what this selection code is achieving.
Attachment #8622816 - Flags: review?(MattN+bmo)
Attachment #8622816 - Flags: review?(MattN+bmo)
Comment on attachment 8622816 [details]
MozReview Request: Bug 1174815 - Caret in capture doorhanger password field is now positioned at the click position offset

Bug 1174815 - Caret in capture doorhanger password field is now positioned at the click position offset
Comment on attachment 8622816 [details]
MozReview Request: Bug 1174815 - Caret in capture doorhanger password field is now positioned at the click position offset

https://reviewboard.mozilla.org/r/11447/#review9887

Ship It!
Attachment #8622816 - Flags: review?(MattN+bmo) → review+
Comment on attachment 8622816 [details]
MozReview Request: Bug 1174815 - Caret in capture doorhanger password field is now positioned at the click position offset

Approval Request Comment
[Feature/regressing bug #]: This should get fixed in the same release as bug 1143903 and bug 1145913 to avoid confusing and churning of the login capture doorhanger UX.
[User impact if declined]: The caret will be in a non-standard position upon focusing the password field.
[Describe test coverage new/current, TreeHerder]: Manual testing on Nightly (email sent to passwords-dev).
[Risks and why]: Fairly straightforward pattern of toggling the field type. Worst case would likely be continued incorrect caret position which wouldn't be a big deal.
[String/UUID change made/needed]: None
Attachment #8622816 - Flags: approval-mozilla-beta?
Attachment #8622816 - Flags: approval-mozilla-aurora?
Blocks: 117527
Blocks: 1175279
No longer blocks: 117527
Rank: 15
Priority: -- → P1
https://hg.mozilla.org/mozilla-central/rev/d2eb65fdebb3
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
QA Contact: kjozwiak
Comment on attachment 8622816 [details]
MozReview Request: Bug 1174815 - Caret in capture doorhanger password field is now positioned at the click position offset

Improve a new feature, taking it for 40.
Attachment #8622816 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Reproduced the original issue using the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-06-16-03-02-01-mozilla-central/

While clicking on the password field, the cursor would be placed at the beginning of the content every time.

Went through verification using the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-06-19-03-02-04-mozilla-central/

- ensured that when you click on the password field, the cursor is placed at the end of the content
- ensured that when you click on the actual password content, the cursor is placed in the correct position
- ensured that all the content under the password field is selected when you tab into the password field
- ensured that the cursor is placed at the end of the content in the password field when it's really long and spans further than the field

OS's Used:

- OSX 10.10.3 x64
- Win 8.1 x64 (VM)
- Ubuntu 14.04.1 x64 (VM)

I also created bug # 1176257 which is a possible trivial issue relating to the cursor not being visible to the user when the password is really long.
I was able to reproduce this issue on Firefox 41.0a1 (2015-06-16) under Windows 7 64-bit.
Verified fixed following the all scenarios from Comment 10 on Firefox 40 Beta 4 (20150713153304) under Windows 7 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.9.5.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.