If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

()

Core
DOM: Core & HTML
RESOLVED FIXED
4 years ago
a year ago

People

(Reporter: DDT, Assigned: jessica)

Tracking

({testcase})

Trunk
mozilla49
testcase
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: btpp-active)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 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

4 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

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

Updated

4 years ago
Keywords: testcase
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

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

Updated

a year ago
Assignee: nobody → jjong
(Assignee)

Comment 3

a year 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

a year ago
Created attachment 8756160 [details] [diff] [review]
patch, v1.
(Assignee)

Comment 5

a year 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 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

a year 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

a year ago
Whiteboard: btpp-active
(Assignee)

Comment 8

a year ago
Created attachment 8757813 [details] [diff] [review]
patch, v2.
Attachment #8756160 - Attachment is obsolete: true
Attachment #8757813 - Flags: review?(bugs)

Updated

a year ago
Attachment #8757813 - Flags: review?(bugs) → review+
(Assignee)

Comment 9

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8dfb9a8461c
Keywords: checkin-needed

Comment 10

a year 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

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

Updated

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