Closed
Bug 926019
Opened 9 years ago
Closed 7 years ago
focus and blur an input element with type of text will trigger change event even if content hasn't changed
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: xulxqqqq, Assigned: jessica)
References
Details
(Keywords: testcase, Whiteboard: btpp-active)
Attachments
(2 files, 1 obsolete file)
630 bytes,
text/html
|
Details | |
4.61 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release) Build ID: 20130910160258 Steps to reproduce: The page code such as follow: <!DOCTYPE html> <html> <head> <meta charset="utf-8" /> </head> <body> <input type="text" value="1"> <script type="text/javascript"> document.getElementsByTagName('input')[0].onfocus = function(){ this.blur(); }; document.getElementsByTagName('input')[0].onchange = function(){ alert('changed'); } </script> </body> </html> Actual results: The problem occurs on Firefox 20.0 (Ubunt 13.04 32bit) / 20.0.1 (Windows 7 64) When click input first time, the alert popups. Then click input again, alert will not show. Expected results: On Firefox 11.0 (windows) and previous version / chrome / ie Clicks input and alert will never show
Summary: focus and blur and input with type of text will trigger change event even if content hasn't changed → focus and blur an input element with type of text will trigger change event even if content hasn't changed
Updated•9 years ago
|
Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core
Comment 1•9 years ago
|
||
I was able to reproduce this on Nightly.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All
Version: 24 Branch → Trunk
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jjong
Assignee | ||
Comment 3•7 years ago
|
||
The reason for this is because blur pre/postHandleEvent is nested in focus pre/postHandleEvent, but mFocusedValue is updated in focus postHandleEvent: 1. preHandleEvent focus 2. preHandleEvent blur -> compare value and mFocusedValue and fire change event if needed 3. postHandleEvent blur 4. postHandleEvent focus -> update mFocusedValue A way to fix this is to update mFocusedValue in focus preHandleEvent. Need to check if this has any side effects.
Assignee | ||
Comment 4•7 years ago
|
||
Assignee | ||
Comment 5•7 years ago
|
||
Comment on attachment 8756160 [details] [diff] [review] patch, v1. Review of attachment 8756160 [details] [diff] [review]: ----------------------------------------------------------------- Hi Olli, I ran try and it seems okay, do you think this fix is good? Thanks.
Attachment #8756160 -
Flags: feedback?(bugs)
Comment 6•7 years ago
|
||
Comment on attachment 8756160 [details] [diff] [review] patch, v1. >+ if (aVisitor.mEvent->mMessage == eFocus && while you're here, want to check that the event is actually trusted. >+ MayFireChangeOnBlur() && >+ !mIsDraggingRange) { // StartRangeThumbDrag already set mFocusedValue >+ GetValue(mFocusedValue); >+ } StartRangeThumbDrag isn't called in PreHandleEvent. Should that part be fixed too? At leas the comment needs to be updated. This all looks like a regression from Bug 722599.
Attachment #8756160 -
Flags: feedback?(bugs) → feedback+
Assignee | ||
Comment 7•7 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #6) > Comment on attachment 8756160 [details] [diff] [review] > patch, v1. > > > >+ if (aVisitor.mEvent->mMessage == eFocus && > while you're here, want to check that the event is actually trusted. Will do. > > >+ MayFireChangeOnBlur() && > >+ !mIsDraggingRange) { // StartRangeThumbDrag already set mFocusedValue > >+ GetValue(mFocusedValue); > >+ } > > StartRangeThumbDrag isn't called in PreHandleEvent. Should that part be > fixed too? > At leas the comment needs to be updated. We still need to check the mIsDraggingRange flag since the order of the events are: - preHandleEvent mousedown - postHandleEvent mousedown -> StartRangeThumbDrag - preHandlEvent focus -> now we set mFocusedValue here - postHandleEvent focus Will update the comment a bit to make it clearer. Thanks. > > > This all looks like a regression from Bug 722599.
Assignee | ||
Updated•7 years ago
|
Whiteboard: btpp-active
Assignee | ||
Comment 8•7 years ago
|
||
Attachment #8756160 -
Attachment is obsolete: true
Attachment #8757813 -
Flags: review?(bugs)
Updated•7 years ago
|
Attachment #8757813 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 9•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8dfb9a8461c
Keywords: checkin-needed
Comment 10•7 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a6e49cbb3a5b focus and blur an input element should not trigger change event if content hasn't changed. r=smaug
Keywords: checkin-needed
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a6e49cbb3a5b
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•