New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 706370 link

Starred by 4 users
Status: Fixed
Owner:
tkent@chromium.org
Closed: Apr 2017
Cc:
█ krajshree@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

onchange event not working correctly when value is changed programmatically when the dom element did have focus

Reported by jcompag...@servoy.com, Mar 29 2017 
Chrome Version       : 58.0.3029.33
OS Version: 10.0
URLs (if applicable) :
Other browsers tested:
  Add OK or FAIL after other browsers where you have tested this issue:
     Safari 5:
  Firefox 4.x:
     IE 7/8/9:

What steps will reproduce the problem?
simple html:
<html>
<head>
<script type="text/javascript">
var counter = 0;
function myFunction() {
  setTimeout(function(){
    document.getElementById("test").value = 'hallo' + (counter++);
  },1000)
}
</script>
</head>
<body>
<input type="test" id="test" name="test" onfocus="myFunction(this)" onchange="console.log('changed:' + document.getElementById('test').value);">
</body>
</html>


if you go into the field and you let it print after a second the first hello0
then go out of the field, go in the field again, then after a second it will become hello1.
Now change the value to hello0. The ondatachange is then not triggered.
If you would alter the value first to something like typing an extra char "hello11" then delete that extra char again, then jump out then onchange is triggered. But the user itself didn't really change anything.

It seems that chrome/chromium does remember some state when the users enters the field. And when it then is changed underneath (like angular or react) then chrome doesn't see that as "the truth from now on". It keeps the old value as the value that a user sees.

I am fine that onchange is not called when i set it the value through scripting. But the change a user does then from that point on should be based on that not the old value. 

What is the expected result?

that the onchange event is based on the new value set through scripting not the old. It shouldn't even call onchange when the user changes the value a bit and the changes it back to the value that the script did put in.

What happens instead of that?

Its constantly using the old value that the user maybe didn't even see or didn't change.

i tested this in other browsers and Edge, Internet Explorer and FireFox are all working as expected. the new scripting value is the value where the onchange is based on.

Please provide any additional information below. Attach a screenshot if
possible.

UserAgentString: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.33 Safari/537.36

Comment 1 by jcompag...@servoy.com, Mar 29 2017 

i am curious about a workaround. 
Can i call something on the element to force chrome to reset its internal state?

Comment 2 by krajshree@chromium.org, Mar 31 2017

Cc: krajshree@chromium.org
Components: Blink>Forms>Text
Labels: Needs-Feedback
Tested the issue on Win-10 using chrome reported version #58.0.3029.33 and latest canary #59.0.3056.0.

Attached a screen cast for reference.

Following are the steps followed to reproduce the issue.
------------
1. Opened the html file in chrome.
2. Entered into the text field and waited for it to print "Hallo0".
3. Came out of the field and entered in the field again, then after a second it changed to "hallo1".
4. Changed the value to "hallo0" and waited for the value to change. 
5. Again altered the value by typing an extra char "hello11" then deleted that extra char again, then jump out and waited for any change inside the field. But no change was triggered.

Reporter@ - Could you please verify the screen cast and please let us know if anything missed from our side. If possible please provide a screen cast of the issue.

Thanks...!!
706370.mp4
560 KB View Download

Comment 3 by jcompag...@servoy.com, Mar 31 2017 

first this is the case that i think caused this issue:
That is really introduced bad behavior
https://bugs.chromium.org/p/chromium/issues/detail?id=92492


But for me on my version: 58.0.3029.41 beta (64-bit) this is still a problem
Second i don't see you looking to the console (there i output when a change is done or not)
so i changed the code a bit to do an alert:

<html>
<head>
<script type="text/javascript">
var counter = 0;
function myFunction() {
  setTimeout(function(){
    document.getElementById("test").value = 'h' + (counter++);
  },1000)
}
</script>
</head>
<body>
<input type="test" id="test" name="test" onfocus="myFunction(this)" onchange="alert('changed:' + document.getElementById('test').value);">
</body>
</html>

So if you load that (refresh) that page
And you go into the field (its an empty field at the moment you jump in)
after 1 second "h0" will popup

If you then delete both chars (so the input is the same as it was when you jumped into the field) and you then tab/jump out of field)
Then an alert should be shown.. But nothing comes up...

The other way around:

refresh the page
go into the field
"h0" will be set after 1 second.
then delete the last char "0" then add it back in so nothing is really changed it still says now "h0" after being "h" for a moment
then jump out
alert is shown! this is wrong nothing is changed the user didn't change the actual value. It still "h0" as it was before.
Project Member

Comment 4 by sheriffbot@chromium.org, Mar 31 2017

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding requester "krajshree@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 5 by ligim...@chromium.org, Mar 31 2017

Labels: Needs-Triage-M58 Needs-Bisect

Comment 6 by tkent@chromium.org, Apr 3 2017

Labels: -Type-Bug -Pri-3 -Needs-Bisect -Needs-Triage-M58 M-58 OS-Android OS-Chrome OS-Linux OS-Mac Pri-1 Type-Bug-Regression
Owner: tkent@chromium.org
Status: Assigned (was: Unconfirmed)
We tried to make Chromium compatible with Firefox on CHANGE event handling. Firefox dispatched a CHANGE event in a case of  Issue 92492 , and it doesn't dispatch a CHANGE event in this case. So we failed to understand the logic in Firefox.

With Firefox and this test case, a CHANGE event is dispatched if we type something before setTimeout() is triggered. I guess the Firefox logic would be:

* On focus
  valueAsOfLastChangeEvent = this.value
  valueBeforeUserEdit = this.value
  userEdit = false

* On value setter
  if (this.value != valueBeforeUserEdit)
    userEdit = true
  this.value = argument of value setter
  valueBeforeUserEdit = this.value

* On blur
  if (this.value != valueBeforeUserEdit)
    userEdit = true
  if (userEdit && this.value != valueAsOfLastChangeEvent)
    dispatchChangeEvent()

Comment 7 by jcompag...@servoy.com, Apr 3 2017 

that fixes the second issue:

focus:
value = "";
valueAsOfLastChangeEvent = ""
valueBeforeUserEdit = ""
userEdit = false

after setter:
value = "h0"
valueAsOfLastChangeEvent = ""
valueBeforeUserEdit = "h0"
userEdit = false

in onblur after i touched the value but still it is "h0"
userEdit stays false so noting happens.


but the first issue:

focus:
value = "";
valueAsOfLastChangeEvent = ""
valueBeforeUserEdit = ""
userEdit = false

after setter:
value = "h0"
valueAsOfLastChangeEvent = ""
valueBeforeUserEdit = "h0"
userEdit = false

then i change the value back to ""
userEdit becomes true ("" != "h0")

but then
this.value != valueAsOfLastChangeEvent is false: "" != ""

which is not correct it should fire an onchange event...

i think that if should be an or:

if (userEdit || this.value != valueAsOfLastChangeEvent)

If it is a user edit because the value is again changed after a setter value or the value is really not the same value as last change event then an onchange should be fired.

Comment 8 by jcompag...@servoy.com, Apr 3 2017 

by the way i see that we now test only on "onblur" but also "enter key" should trigger the onchange (or not) right?

Comment 9 by jcompag...@servoy.com, Apr 3 2017 

hmm changing that to an || makes the second issue "fail" because then we get that userEdit is still false but "h0" != "" is then true...

so thats still not it and adding a extra test like && this.value != valueBeforeUserEdit  will work to fix that and i think userEdit will be true on the setter case? so the firefox behavior of that other case is then also still working?

Comment 10 by tkent@chromium.org, Apr 4 2017 

Probably we just need:

  if (userEdit)
    dispatchChangeEvent()

on blur.  'valueAsOfLastChangeEvent' isn't necessary.

> by the way i see that we now test only on "onblur" but also "enter key" should trigger the onchange (or not) right?

Right.  It should work like blur-then-focus.

Comment 11 by tkent@chromium.org, Apr 4 2017 

>  if (userEdit)
>    dispatchChangeEvent()

Looks it doesn't work :-D

I investigated Firefox behavior again.  Summary:


    A) Focus, Timeout, Blur -> No CHANGE
    B) Focus, Timeout, Emptify, Blur -> CHANGE
    C) Focus, Timeout, Edit and revert, Blur -> No CHANGE
    D) Focus, Type anything, Timeout, Blur -> CHANGE
    E) Focus, Type 'h0', Timeout, Blur -> CHANGE
    F) Focus, Type anything, Timeout, Emptify, Blur -> No CHANGE
    G) Focus, Edit and revert, Timeout, Blur -> No CHANGE 

It seems Firefox just compares the value before the first user edit and the value just after the last user edit.

Comment 12 by tkent@chromium.org, Apr 4 2017

Status: Started (was: Assigned)

Comment 15 by tkent@chromium.org, Apr 5 2017

Labels: -M-58 M-59
We need multiple non-trivial CLs to fix this. Merging them to M58 isn't reasonable, and we aim to fix this in M59.
Project Member

Comment 16 by bugdroid1@chromium.org, Apr 5 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4397adcd1b8e9e667dca4f14bab1453ce146641f

commit 4397adcd1b8e9e667dca4f14bab1453ce146641f
Author: tkent <tkent@chromium.org>
Date: Wed Apr 05 07:18:07 2017

Remove lazy evaluation of HTMLTextAreaElement::value.

Remove it for cleaner code, and preparation to fix  crbug.com/706370 .

The lazy evaluation isn't very helpful because children of innerEditor is
typically 0 or one Text node and an optional trailing <br>. In such case,
innerEditorValue() can just return Text::data(), and it's a cheap operation.

BUG= 706370 

Review-Url: https://codereview.chromium.org/2803483005
Cr-Commit-Position: refs/heads/master@{#461998}

[modify] https://crrev.com/4397adcd1b8e9e667dca4f14bab1453ce146641f/third_party/WebKit/Source/core/html/HTMLTextAreaElement.cpp
[modify] https://crrev.com/4397adcd1b8e9e667dca4f14bab1453ce146641f/third_party/WebKit/Source/core/html/HTMLTextAreaElement.h
[modify] https://crrev.com/4397adcd1b8e9e667dca4f14bab1453ce146641f/third_party/WebKit/Source/core/html/TextControlElement.cpp

Comment 17 by tkent@chromium.org, Apr 6 2017 

> It seems Firefox just compares the value before the first user edit and the value just after the last user edit.

A counter example:

  H) Focus, Type 'a', input.value='', Blur => no change event.

- We should compare the value before the first user edit and the value on blur
- If the value just after any user edit is same as the value before the first user edit, we should clear the value before the first user edit.  (For case G).
Project Member

Comment 18 by bugdroid1@chromium.org, Apr 6 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90b048661a3949cd38ef690052d6930c230b4100

commit 90b048661a3949cd38ef690052d6930c230b4100
Author: tkent <tkent@chromium.org>
Date: Thu Apr 06 04:13:43 2017

Update text field 'change' event logic to match to Firefox.

We tried to do so by r430531, but it was incomplete.
We investigated Firefox behavior in many more cases, and our understanding
is that:

A) Initialize last-value with null.
B) On every user edit, set the value before the edit to last-value if it is null.
C) On every user edit, set last-value to null if it and the new value are same.
D) Dispatch CHANGE event if last-value is not null and it and the current value
  are not same.

TextControlElement::m_valueBeforeFirstUserEdit represents last-value.
TextControlElement::setValueBeforeFirstUserEditIfNotSet() represents B, and
TestControlElement::checkIfValueWasReverted() represents C.  This CL adds them to
every code path updating value by user edit.

BUG= 706370 

Review-Url: https://codereview.chromium.org/2796853002
Cr-Commit-Position: refs/heads/master@{#462349}

[modify] https://crrev.com/90b048661a3949cd38ef690052d6930c230b4100/third_party/WebKit/LayoutTests/fast/forms/textfield-change-event.html
[modify] https://crrev.com/90b048661a3949cd38ef690052d6930c230b4100/third_party/WebKit/Source/core/html/HTMLInputElement.cpp
[modify] https://crrev.com/90b048661a3949cd38ef690052d6930c230b4100/third_party/WebKit/Source/core/html/HTMLInputElement.h
[modify] https://crrev.com/90b048661a3949cd38ef690052d6930c230b4100/third_party/WebKit/Source/core/html/HTMLTextAreaElement.cpp
[modify] https://crrev.com/90b048661a3949cd38ef690052d6930c230b4100/third_party/WebKit/Source/core/html/TextControlElement.cpp
[modify] https://crrev.com/90b048661a3949cd38ef690052d6930c230b4100/third_party/WebKit/Source/core/html/TextControlElement.h
[modify] https://crrev.com/90b048661a3949cd38ef690052d6930c230b4100/third_party/WebKit/Source/core/html/forms/FileInputType.cpp
[modify] https://crrev.com/90b048661a3949cd38ef690052d6930c230b4100/third_party/WebKit/Source/core/html/forms/InputType.cpp
[modify] https://crrev.com/90b048661a3949cd38ef690052d6930c230b4100/third_party/WebKit/Source/core/html/forms/MultipleFieldsTemporalInputTypeView.cpp
[modify] https://crrev.com/90b048661a3949cd38ef690052d6930c230b4100/third_party/WebKit/Source/core/html/forms/TextFieldInputType.cpp

Comment 19 by tkent@chromium.org, Apr 6 2017

Status: Fixed (was: Started)

Comment 20 by krajshree@chromium.org, Apr 11 2017

Labels: TE-Verified-M59 TE-Verified-59.0.3067.0 TE-Verified-59.0.3067.6
Verified the issue on Windows 10 using chrome version #59.0.3067.6 and Mac 10.12.3 and Ubuntu 14.04 using chrome version #59.0.3067.0(due to non-availability of chrome dev build #59.0.3067.6 for OS-Mac and OS-Linux) as per comment #3.

Observed that onchange event was based on the new value set through scripting not the old as expected. Hence, the fix is working as expected.

Attaching screen cast for reference.

Hence, adding the verified labels.

Thanks...!!
706370.mp4
503 KB View Download

Comment 21 by tkent@chromium.org, Apr 13 2017 

 Issue 710949  has been merged into this issue.
►

Sign in to add a comment