commonDialog's default button code doesn't seem to work anymore. see bug 300227 comment 3

Should this be "accept"? if (dButton != "accpet")

yeah, typo.

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 on attachment 192861 [details] [diff] [review] patch (toolkit & xpfe) leaving it as a switch is fine, but the second part should be addressed

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

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