Closed Bug 1077308 Opened 5 years ago Closed 5 years ago
Autocomplete of username/password should generate change event
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.
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 on the attachment details page. I would suggest Olli Pettay from .  https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_reviewed  https://wiki.mozilla.org/Modules/Core#Document_Object_Model
Flags: qe-verify? → qe-verify-
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+
I've updated the patch to fix the indentation.
Try push: https://tbpl.mozilla.org/?tree=Try&rev=cacb25be87f8 Thanks
Assignee: nobody → mbest
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8500674 - Flags: review+
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: https://mxr.mozilla.org/mozilla-central/source/content/html/content/test/test_bug388558.html?force=1 (content/html/content/test/test_bug388558.html) You can run the test with: ./mach mochitest-plain content/html/content/test/test_bug388558.html Can you try fix the test?
Thanks for the feedback. I've updated the test.
Did the updated patch get run through Try to verify that the test failure is fixed?
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.
OK, thanks. I've pushed to Try server again: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=a831e8f494d7 I also updated the commit message and timestamp in https://hg.mozilla.org/try/rev/0d79dd9a6338 so that can be used for the push to an integration branch.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Duplicate of this bug: 87943
You need to log in before you can comment on or make changes to this bug.