Bug 6731 – REGRESSION: change event fires at load time for checked items

• Status: VERIFIED
• Resolution: FIXED
• Component: Forms

[Dennis Rowe]

It looks like if you have an onChange in a input and that button is "checked" the onChange code is run when the page is loaded.  If I was to hazard a guess, the button is set up with the onChange hook attached to the button before the button is checked on the initial page load.

[Dennis Rowe]

Created attachment 5871 [details]
Example of the given bug

This html causes the page to continously reload the page.  It is a reduced version of the bug.

[Dennis Rowe]

The error occurs on the latest nightly build, but does not occur on the current stable version for Mac OSX 10.4, Version 2.0.3 (417.8).

[Joost de Valk (AlthA)]

This is indeed a regression, marking as such, adding keyword, making P1 because of it. This is also a HitListCandidate. I'm guessing component is forms, but not sure, could be javascript too, wouldn't know where to solve this.

[Darin Adler]

Created attachment 5936 [details]
fixes this by only sending the change event if the element is in a document

[Adele Peterson]

Comment on attachment 5936 [details]
fixes this by only sending the change event if the element is in a document

r=me

[Dennis Rowe]

I downloaded and compile webkit today.  The patch fixed the problem for the given test case, but the problem still comes up a nearly identical test case.  I will reopen and attach second test case.

[Dennis Rowe]

Created attachment 5945 [details]
Another example of the bug

Here is an example of the bug showing up in a nearly identical case.

[Dennis Rowe]

The second test case is a bit strange.  The name of the inputs are the same and when two inputs with the same name are "checked" it causes the onChange to fire.  I don't know if this is the way it should work or not.  It seems a bit strange that the onChange would be tied to the name instead of the actual input button, but I am really at a loss here.

[Darin Adler]

In the new example, the change event is firing because of the fact that parsing the second item causes the first one to be unchecked. It's the "unchecking" of the first item (which is already in the document) that is the change being reported.

[Darin Adler]

Comment on attachment 5936 [details]
fixes this by only sending the change event if the element is in a document

Clearing the review flag off this already-landed patch so this bug doesn't show up in the "ready to commit" queue.

[Darin Adler]

Created attachment 5947 [details]
patch to fix the remaining problem

[Maciej Stachowiak]

Comment on attachment 5947 [details]
patch to fix the remaining problem

r=me

[Dennis Rowe]

I downloaded the source and compiled it and tested it against the reduced test
cases and also against the case that the reduced test came from.  In both cases
the new patch worked as it should. Thanks!

[Eric Seidel (no email)]

Removing Regression keyword from bugs already fixed.