51796 - Right clicking on XUL radio button checks button without unchecking others

Status: RESOLVED FIXED

(Core :: XUL, defect, P3)

Description

[Blake Ross]

Build ID: (just pulled)

Steps to Reproduce:

(1) Open Prefs
(2) Go to the Navigator pane
(3) Right click on all of the radio buttons in When Navigator starts up, Display

Note that all the buttons get checked, but nothing gets unchecked.  This might 
be related to a fix I recall ben checking in for bug 30878 to fix radio buttons 
and checkboxes (but then, I also have a recollection of him backing out his 
change...and checkboxes still check on right-click)

Comment 1

[Peter Trudelle]

Didn't Timeless contribute a patch that Ben needs to review?  That bug is still
nsbeta3-, so this is probably a dup, and is in any case ->future.

Comment 2

[Blake Ross]

I don't think so (re: this being a dup), but maybe.  iirc, right clicking on a 
radio button used to still check it (which is what that bug covers), but would 
still uncheck the other radio buttons in that radiogroup.  This no longer works 
properly.  Another incorrect behavior is that if a radio button is already 
checked and you right click it, it will uncheck (in other words, right click 
just toggles the checked/unchecked states without having regard for the rest of 
the radiogroup).

In any case, future is fine...

Comment 3

[Peter Trudelle]

I guess I assumed that this misbehavior would go away once the issues described
in bug 30878 were resolved, but that's why I futured it rather than resolving,
so we can sort out the details when we have more time.

Comment 4

[Fabian Guisset]

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

Comment 5

[Fabian Guisset]

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

Comment 6

[John Morrison]

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

Comment 7

[Fabian Guisset]

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

Comment 8

[Fabian Guisset]

I have read bug 30878 and it doesn't help this. Bug 30878 is about oncommand and
onclick, while this is about right-clicking on radio buttons. I have a very easy
and safe fix for this, which apparently works for me.. It consists in adding a
second "if" to the click handler like this :

if (!event.target.disabled &&
  event.target.localName == "radio" &&
  event.button == 1)
    this.selectedItem = event.target;
else if (!event.target.disabled &&
  event.target.localName == "radio" &&
  event.button == 3)
    this.selectedItem = event.target;

Note that we could eventually add a "|| event.button == 3" in the first if to
avoid a second if, which could be bad for performance, but I'm not sure that
works, i'll test it when I get back home.
This way, right-clicking on radio buttons acts just the same as left-clicking.
Now if we don't want to use the right-click at all, then I don't know how to do
it, but I'm happy with the right-click = left-click.
Fabian.

Comment 9

[Peter "jag" Annema]

I don't think right-clicking a checkbox or radiobutton should modify the state.

The right fix in that case would be to make xulBindings.xml#checkbox's click
handler check for button 1 (left-click) to be used before changing state.

Comment 10

[Peter Trudelle]

->ben

Comment 11

[Matthew T (active 1999-2002)]

No, don't make the secondary button do the same thing the primary button does -- 
all that will do is confuse a complete novice user about which button is which. 
(And native Windows radio buttons don't respond to the secondary mouse button, 
either.)

Comment 12

[Blake Ross]

Taking....

Comment 13

[Blake Ross]

Attachment: [patch] fix radiobuttons/checkboxes

Comment 14

[Blake Ross]

Attachment: [patch] remove redundant disabled checks

Comment 15

[Blake Ross]

cc'ing alec for sr

Comment 16

[Alec Flett]

coolness.
sr=alecf

Comment 17

[timeless]

is |if (event.target.localName == "radio")| essential?

Comment 18

[timeless]

r=timeless, hoping that selecteditem doesn't catch radiobuttons in nested 
radiogroups.

Comment 19

[Blake Ross]

Fix checked in.