Summary: | REGRESSION: check box onchange event doesn't fire (radio buttons do not get disabled in Bugzilla) | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||||||
Component: | Forms | Assignee: | Adele Peterson <adele> | ||||||||||
Status: | VERIFIED FIXED | ||||||||||||
Severity: | Normal | CC: | bdakin | ||||||||||
Priority: | P1 | ||||||||||||
Version: | 420+ | ||||||||||||
Hardware: | Mac | ||||||||||||
OS: | OS X 10.4 | ||||||||||||
Attachments: |
|
Description
Alexey Proskuryakov
2005-10-23 02:59:18 PDT
I think the checkbox's onchange event isn't getting fired. Created attachment 4693 [details]
Patch to call onChange() after a radio button or a checkbox is checked
Comment on attachment 4693 [details]
Patch to call onChange() after a radio button or a checkbox is checked
Dave- I saw that in onchange, you had this comment: "// ### make this work
with new form events architecture" ... So I wasn't sure if this is too limited
of a fix. But it definitely cures the symptom.
Created attachment 4694 [details]
layout test for checkbox onchange.
attaching layout test
Not sure what the layout test is trying to illustrate. onchange doesn't fire in Safari 2.0 or Firefox for that test case. Comment on attachment 4693 [details]
Patch to call onChange() after a radio button or a checkbox is checked
I don't think the timing of onchange is right here.
weird, I see onchange fire (and the test case pass) for Safari 2.0.2 and FF, but not on TOT. Comment on attachment 4693 [details]
Patch to call onChange() after a radio button or a checkbox is checked
Dave, the test case *does* fire an onchange event in Firefox. You should
reconsider. I think this patch is correct.
This patch needs copyright notice, test case included in patch, and ChangeLog entry to comply with WebKit patch submission guidelines. Otherwise the change looks correct to me (I can confirm that the test case passes in Firefox and Safari 2.0.2 but fails in TOT Safari). Comment on attachment 4693 [details]
Patch to call onChange() after a radio button or a checkbox is checked
I hate to r- a correct-looking one-liner over technicalities but them's the
rules. In case of emergency we'd probably merge this anyway.
Created attachment 5073 [details]
updated patch
here's an updated patch w/ changelog
Created attachment 5074 [details]
new testcase
also tests radio buttons now too
|