254238 - <input type="radio"> fires onChange twice for each click

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Mats Palmgren (inactive)]

Followup from bug 253299 comment 7:

Here is another problem that could be a regression from bug 17602.
Clicking the radio buttons, I see:
in Mozilla 1.7:   focus, click, change
in 2004-07-26-07: focus, click, change, change

Comment 1

[Boris Zbarsky [:bzbarsky]]

This is a pretty major DOM bug to have around....  See bug 275413 comment 3 for an 
idea of why this is happening.  Aaron, Brian, do you have any idea what's going on?

Comment 2

[Asa Dotzler [:asa]]

Aaron, time is getting pretty short for Alpha6. Can you take a look at this soon?

Comment 3

[Aaron Leventhal]

I'll try, but I'm on vacation. If possible can someone else take a look?

Comment 4

[Asa Dotzler [:asa]]

Johnny, are you available to look at this for 1.8a6?

Comment 5

[Boris Zbarsky [:bzbarsky]]

It looks like this broke between 2004-04-28-08 and 2004-05-01-07.  That's when
Brian landed bug 60212; of the checkins in that range, it's the most likely
culprit by far.

Checkin list:

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-04-28+08%3A00%3A00&maxdate=2004-05-01+07%3A00%3A00&cvsroot=%2Fcvsroot

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Possible patch

It looks to me like all munging of the BF_CHECKED_IS_TOGGLED flag, of the
selectedRadioButton variable, etc, is conditioned on us not being in capture
and not being in the system event group, except this little chunk of code
(which happens to be where we fire onchange).  The patch fixes the testcase,
but I'm not completely sure how correct it is, unfortunately...

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 170392 [details] [diff] [review]
Possible patch

Seems right to me. sr=jst

Comment 8

[Daniel Brooks [:db48x]]

*** Bug 275413 has been marked as a duplicate of this bug. ***

Comment 9

[Brian Ryner (not reading)]

Attachment: alternate fix

I like this way better... makes us completely consistent.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 170685 [details] [diff] [review]
alternate fix

I'm not sure "toggle" is a good word, since this is the event that causes
inputs to submit forms too, and all.  Maybe "outerActivateEvent", with a
comment that this is the event that should cause the input's "default action",
whatever that is, to be triggered?

r=bzbarsky for the code. anyway, no matter how we resolve this variable naming
issue.

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 170685 [details] [diff] [review]
alternate fix

sr=jst

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 170685 [details] [diff] [review]
alternate fix

a=asa (on behalf of drivers) for checkin to 1.8a6

Comment 13

[Brian Ryner (not reading)]

checked in.