Closed Bug 1077308 Opened 5 years ago Closed 5 years ago

Autocomplete of username/password should generate change event


(Toolkit :: Password Manager, defect)

33 Branch
Not set





(Reporter: mbest, Assigned: mbest)



(Keywords: site-compat)


(1 file, 2 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/37.0.2062.120 Safari/537.36

Steps to reproduce:

Open a page with a username/password form for which you've already saved a password. Or select a saved username from the drop-down after the page is loaded.

Actual results:

The username and password are filled in and "input" events are fired, but not "change" events.

Expected results:

"change" events should be fired in addition to "input". This is what Internet Explorere and Chrome do.
See Also: → 1025483, 1077304
Flags: qe-verify?
Possibly it would make sense to rename "ShouldBlur" to "IsFocused", but the main logic of this patch is what is important.
Thanks for the patch Michael! Would you like feedback/review on the patch? If so, you should set the appropriate flag[1] on the attachment details page. I would suggest Olli Pettay from [2].

Flags: qe-verify? → qe-verify-
Attachment #8500102 - Flags: review?(bugs)
Comment on attachment 8500102 [details] [diff] [review]
If a field is auto-filled while not in focus, fire a change event immediately

Review of attachment 8500102 [details] [diff] [review]:

The password manager test looks good to me. I'll let a DOM peer review content/ and whether this makes sense with web specs. The spec says:
"The change event fires when the value is committed, if that makes sense for the control, or else when the control loses focus. In all cases, the input event comes before the corresponding change event (if any)."

So this seems reasonable to me.

::: content/html/content/src/HTMLInputElement.cpp
@@ +2409,2 @@
>                                                static_cast<nsIDOMHTMLInputElement*>(this),
>                                                NS_LITERAL_STRING("input"), true,

Nit: fix the indentation to align the arguments.
Attachment #8500102 - Flags: review+
Comment on attachment 8500102 [details] [diff] [review]
If a field is auto-filled while not in focus, fire a change event immediately

>-  return nsContentUtils::DispatchTrustedEvent(OwnerDoc(),
>+  nsContentUtils::DispatchTrustedEvent(OwnerDoc(),
>                                               static_cast<nsIDOMHTMLInputElement*>(this),
>                                               NS_LITERAL_STRING("input"), true,
>                                               true);
Indent the rest of the params.
Attachment #8500102 - Flags: review?(bugs) → review+
Attachment #8500102 - Attachment is obsolete: true
Attachment #8500674 - Flags: review?(bugs)
I've updated the patch to fix the indentation.
Attachment #8500674 - Flags: review?(bugs) → review+
Try push:

Assignee: nobody → mbest
Ever confirmed: true
Keywords: checkin-needed
Hi Michael,

The try server run is showing a test failure:
TEST-UNEXPECTED-FAIL | /tests/content/html/content/test/test_bug388558.html | Change event dispatched when input element doesn't have focus. - got 2, expected 1

The test file is at:

You can run the test with:
./mach mochitest-plain content/html/content/test/test_bug388558.html

Can you try fix the test?
Flags: needinfo?(mbest)
Keywords: checkin-needed
Attachment #8500674 - Attachment is obsolete: true
Thanks for the feedback. I've updated the test.
Flags: needinfo?(mbest)
Keywords: checkin-needed
Did the updated patch get run through Try to verify that the test failure is fixed?
Keywords: checkin-needed
I'm not able to run through the whole test suite in my development environment (using the Firefox build VM). But the change I made does make the affected test pass.
Flags: needinfo?(MattN+bmo)
OK, thanks. I've pushed to Try server again:

I also updated the commit message and timestamp in so that can be used for the push to an integration branch.
Flags: needinfo?(MattN+bmo)
Keywords: checkin-needed
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla36
Iteration: --- → 36.1
You need to log in before you can comment on or make changes to this bug.