304879 - regression: commonDialog's default button code doesn't work anymore

Status: RESOLVED FIXED

(Core :: XUL, defect, P1)

Description

[Asaf Romano (gone)]

commonDialog's default button code doesn't seem to work anymore.

see bug 300227 comment 3

Comment 1

[Asaf Romano (gone)]

Attachment: patch (toolkit & xpfe)

Comment 2

[Ed Lee :Mardak]

Should this be "accept"?

if (dButton != "accpet")

Comment 3

[Asaf Romano (gone)]

yeah, typo.

Comment 4

[[e-mail]]

Comment on attachment 192861 [details] [diff] [review]
patch (toolkit & xpfe)

>     switch (defaultButton) {
>       case 3:
>-        dButton = document.documentElement.getButton("extra2");
>+        dButton = "extra2";
>         break;
>       case 2:
>-        dButton = document.documentElement.getButton("extra1");
>+        dButton = "extra1";
>         break;
>       case 1:
>-        dButton = document.documentElement.getButton("cancel");
>+        dButton = "cancel";
>         break;
>       default:
>       case 0:
>-        dButton = document.documentElement.getButton("accept");
>+        dButton = "accept";
>         break;
>     }
This switch could probably be simplified into an array lookup
(nsPromptService.cpp only passes values of 0 to 3).

>+    // Set the default button if necessary
>+    if (dButton != "accpet")
I don't see the point of this extra condition, are we really going to affect
the performance of launching a dialog by that much?

Comment 5

[Mike Connor [:mconnor]]

Comment on attachment 192861 [details] [diff] [review]
patch (toolkit & xpfe)

leaving it as a switch is fine, but the second part should be addressed

Comment 6

[Mike Connor [:mconnor]]

plussing, has patch with r+sr, just needs review, and is a regression

Comment 7

[Asaf Romano (gone)]

Attachment: patch for checkin

Comment 8

[Asaf Romano (gone)]

Trunk:
Checking in toolkit/content/commonDialog.js;
/cvsroot/mozilla/toolkit/content/commonDialog.js,v  <--  commonDialog.js
new revision: 1.10; previous revision: 1.9
done
Checking in xpfe/global/resources/content/commonDialog.js;
/cvsroot/mozilla/xpfe/global/resources/content/commonDialog.js,v  <-- 
commonDialog.js
new revision: 1.57; previous revision: 1.56
done

1.8 Branch:
Checking in toolkit/content/commonDialog.js;
/cvsroot/mozilla/toolkit/content/commonDialog.js,v  <--  commonDialog.js
new revision: 1.9.2.1; previous revision: 1.9
done
Checking in xpfe/global/resources/content/commonDialog.js;
/cvsroot/mozilla/xpfe/global/resources/content/commonDialog.js,v  <-- 
commonDialog.js
new revision: 1.56.8.1; previous revision: 1.56
done