focus and blur an input element with type of text will trigger change event even if content hasn't changed

RESOLVED FIXED in Firefox 49

Status

()

Product:
Core
Component:
DOM: Core & HTML
Status:
RESOLVED FIXED
Reported:
5 years ago
Modified:
3 years ago

People

(Reporter: xulxqqqq, Assigned: jessica)

Tracking

({testcase})

Version:
Trunk
Target:
mozilla49
Keywords:
testcase
Points:
---
Duplicates:
1173341

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: btpp-active)

Attachments

(2 attachments, 1 obsolete attachment)

very simplistic showcase to highlight the issue
630 bytes, text/html
Details
patch, v2.
4.61 KB, patch
smaug
: review+
Details | Diff | Splinter Review
(Reporter)

Description

•
5 years ago 
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
 (Reporter)

Updated

•
5 years ago
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

•
5 years ago
Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core

Updated

•
5 years ago
Keywords: testcase

Comment 1

•
5 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

Comment 2

•
3 years ago 
Created attachment 8694881 [details]
very simplistic showcase to highlight the issue
 (Assignee)

Updated

•
3 years ago
Assignee: nobody → jjong
 (Assignee)

Comment 3

•
3 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 5

•
3 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

•
3 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

•
3 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

•
3 years ago
Whiteboard: btpp-active
 (Assignee)

Comment 8

•
3 years ago 
Created attachment 8757813 [details] [diff] [review]
patch, v2.
Attachment #8756160 - Attachment is obsolete: true
Attachment #8757813 - Flags: review?(bugs)
Attachment #8757813 - Flags: review?(bugs) → review+

Comment 10

•
3 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

•
3 years ago
bugherder 
https://hg.mozilla.org/mozilla-central/rev/a6e49cbb3a5b
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
 (Assignee)

Updated

•
3 years ago
Duplicate of this bug: 1173341
You need to log in before you can comment on or make changes to this bug.