Bug 5465

Summary: REGRESSION: check box onchange event doesn't fire (radio buttons do not get disabled in Bugzilla)
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: FormsAssignee: Adele Peterson <adele>
Status: VERIFIED FIXED    
Severity: Normal CC: bdakin
Priority: P1    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Patch to call onChange() after a radio button or a checkbox is checked
mjs: review-
layout test for checkbox onchange.
none
updated patch
mjs: review+
new testcase none

Description Alexey Proskuryakov 2005-10-23 02:59:18 PDT
Radio buttons for content-type are supposed to be disabled for patches in Bugzilla, but they aren't.

Steps to reproduce:
1. Click "Create a New Attachment" link for this (any) bug report.
2. Click "patch" checkbox.

Expected results: the radio buttons below should get disabled
Results: they aren't.

Works fine in 2.0.1. This has regressed quite a while ago (a couple of months, perhaps).
Comment 1 Adele Peterson 2005-11-15 15:37:24 PST
I think the checkbox's onchange event isn't getting fired.
Comment 2 Adele Peterson 2005-11-15 16:17:42 PST
Created attachment 4693 [details]
Patch to call onChange() after a radio button or a checkbox is checked
Comment 3 Adele Peterson 2005-11-15 16:19:59 PST
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.
Comment 4 Adele Peterson 2005-11-15 16:24:57 PST
Created attachment 4694 [details]
layout test for checkbox onchange.

attaching layout test
Comment 5 Dave Hyatt 2005-11-15 17:15:59 PST
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 6 Dave Hyatt 2005-11-15 17:16:19 PST
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.
Comment 7 Adele Peterson 2005-11-15 17:21:04 PST
weird, I see onchange fire (and the test case pass) for Safari 2.0.2 and FF, but not on TOT.
Comment 8 Darin Adler 2005-12-10 08:12:53 PST
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.
Comment 9 Maciej Stachowiak 2005-12-13 22:11:03 PST
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 10 Maciej Stachowiak 2005-12-13 22:11:45 PST
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.
Comment 11 Adele Peterson 2005-12-13 23:40:00 PST
Created attachment 5073 [details]
updated patch

here's an updated patch w/ changelog
Comment 12 Adele Peterson 2005-12-13 23:41:00 PST
Created attachment 5074 [details]
new testcase

also tests radio buttons now too