Closed
Bug 926019
Opened 12 years ago
Closed 9 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•12 years ago
|
Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core
|
||
Comment 1•11 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•9 years ago
|
Assignee: nobody → jjong
|
|
Assignee | |
Comment 3•9 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•9 years ago
|
||
|
|
Assignee | |
Comment 5•9 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•9 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•9 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•9 years ago
|
Whiteboard: btpp-active
|
|
Assignee | |
Comment 8•9 years ago
|
||
Attachment #8756160 -
Attachment is obsolete: true
Attachment #8757813 -
Flags: review?(bugs)
|
|
||
Updated•9 years ago
|
Attachment #8757813 -
Flags: review?(bugs) → review+
|
|
Assignee | |
Comment 9•9 years ago
|
||
Keywords: checkin-needed
|
||
Comment 10•9 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•9 years ago
|
||
| bugherder | ||
Status: NEW → RESOLVED
Closed: 9 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
•