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:

Forms

Assignee:

Adele Peterson <adele>

Status:

VERIFIED FIXED

Severity:

Normal

CC:

bdakin

Priority:

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

Alexey Proskuryakov

Reported 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).

Attachments
Patch to call onChange() after a radio button or a checkbox is checked (597 bytes, patch) 2005-11-15 16:17 PST, Adele Peterson	mjs: review-	Details Formatted Diff Diff
layout test for checkbox onchange. (323 bytes, text/html) 2005-11-15 16:24 PST, Adele Peterson	no flags	Details
updated patch (2.83 KB, patch) 2005-12-13 23:40 PST, Adele Peterson	mjs: review+	Details Formatted Diff Diff
new testcase (690 bytes, text/html) 2005-12-13 23:41 PST, Adele Peterson	no flags	Details
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Adele Peterson

Comment 1 2005-11-15 15:37:24 PST

I think the checkbox's onchange event isn't getting fired.

Adele Peterson

Comment 2 2005-11-15 16:17:42 PST

Created attachment 4693 [details] Patch to call onChange() after a radio button or a checkbox is checked

Adele Peterson

Comment 3 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.

Adele Peterson

Comment 4 2005-11-15 16:24:57 PST

Created attachment 4694 [details] layout test for checkbox onchange. attaching layout test

Dave Hyatt

Comment 5 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.

Dave Hyatt

Comment 6 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.

Adele Peterson

Comment 7 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.

Darin Adler

Comment 8 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.

Maciej Stachowiak

Comment 9 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).

Maciej Stachowiak

Comment 10 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.

Adele Peterson

Comment 11 2005-12-13 23:40:00 PST

Created attachment 5073 [details] updated patch here's an updated patch w/ changelog

Adele Peterson

Comment 12 2005-12-13 23:41:00 PST

Created attachment 5074 [details] new testcase also tests radio buttons now too

Note You need to log in before you can comment on or make changes to this bug.