112803 - print dialog: radio buttons should use oncommand="..."

Status: VERIFIED FIXED

(Core :: Printing: Output, defect)

Description

[tan]

Currently printdialog.xul uses onclick to capture events on the radio buttons,
however, for a keyboard navigator, these events will never get fired.

1. Open Print dialog
2. Tab to you get to Print Range box
3. Press down to Page radio
4. Notice From and To text boxes are not enabled

Comment 1

[tan]

--- /home/mozilla096/usr/lib/mozilla/chrome/content/global/printdialog.xul  Fri
+++ content/global/printdialog.xul  Fri Nov 30 18:38:53 2001
@@ -81,15 +81,15 @@
       <caption label="&printrangeGroup.label;"/>

       <radiogroup id="printrangeGroup">
-        <radio id="allpagesRadio" label="&allpagesRadio.label;" oncommand="doPr
+        <radio id="allpagesRadio" label="&allpagesRadio.label;" onclick="doPrin
         <hbox align="center">
-          <radio id="rangeRadio" label="&rangeRadio.label;" oncommand="doPrintR
+          <radio id="rangeRadio" label="&rangeRadio.label;" onclick="doPrintRan
           <label id="frompageLabel" control="frompageInput" value="&frompageInp
           <textbox id="frompageInput" style="width:5em;" onkeyup="checkInteger(
           <label id="topageLabel" control="topageInput" value="&topageInput.lab
           <textbox id="topageInput" style="width:5em;" onkeyup="checkInteger(th
         </hbox>
-        <radio id="selectionRadio" label="&selectionRadio.label;" oncommand = "
+        <radio id="selectionRadio" label="&selectionRadio.label;" onclick = "do
       </radiogroup>
     </groupbox>

Comment 2

[Boris Zbarsky [:bzbarsky]]

roland

Comment 3

[Roland Mainz]

bz:
Is there any XUL/JS guru who can confirm that s/onclick/oncommand/ is the
"right" thing to do ?

Comment 4

[Boris Zbarsky [:bzbarsky]]

just ask blake on irc.  btw, timeless was saying that using onclick instead of
oncommand in XUL is an offense worth being shot for.  So yes, this should use
oncommand.  :)

Comment 5

[Asko Tontti]

hmm, I fixed some of those but obviously not all of them :(

I have some other fixes, comments, cleanup changes in my tree for dialogs.
I could submit them here?

Comment 6

[Roland Mainz]

Asko Tontti wrote:
> I have some other fixes, comments, cleanup changes in my tree for dialogs.
> I could submit them here?

If the patches are for the print stuff "only" and/or if they are not "too" big
then yes... othwise it may be better to open a new bug and make this bug
"depend" on your one...

Comment 7

[Asko Tontti]

Changes are only for print dialog and print job dialog.

I am now making test build but unfortantely it takes some time. After I have
tested my patch I will attach it.

Comment 8

[Asko Tontti]

Attachment: patch for replacing onclick + cleanup, more comments, removed dead code, ...

I have done some testing with it but it would be helpful if somebody else
would also do testing.

Comment 9

[Roland Mainz]

Comment on attachment 59977 [details] [diff] [review]
patch for replacing onclick + cleanup, more comments, removed dead code, ...

r=[e-mail]

Comment 10

[Roland Mainz]

Comment on attachment 59977 [details] [diff] [review]
patch for replacing onclick + cleanup, more comments, removed dead code, ...

rods:
I need a moa= from you for the patch, please ...

Comment 11

[rods (gone)]

I have incorporated the fix into Bug 113917

Comment 12

[Roland Mainz]

Reporter:
bug 113917 has been marked as FIXED - is this issue fixed for you, too - or is
there still work required ?

Comment 13

[Roland Mainz]

Seems to be fixed.

Marking bug as FIXED, please reopen when this bug is still an issue for you...

Reporter:
Can you please verify this bug ? Thanks!

Comment 14

[sujay]

Roland please verify...thanks!

Comment 15

[sujay]

tan, please verify...thanks...

Comment 16

[sujay]

marking verified. REOPEN if still not fixed...