Skip to content

Commit

Permalink
Bug 1925401: Require BUTTON_NONE in nsIPrompt for button-less dialogs…
Browse files Browse the repository at this point in the history
… r=Gijs,necko-reviewers

This will prevent devs from accidentally requesting no buttons, since
the only way to dismiss such a prompt is with an intentional call to
prompt.close().  A request that indicates that button 0's label is
the empty string previously indicated button 0 is hidden, which means no
buttons.  Now, that case is an exception unless BUTTON_NONE is set.

Differential Revision: https://phabricator.services.mozilla.com/D226081

UltraBlame original commit: 80b0b1f6f80610d09dfd79275e7ededf2089205e
  • Loading branch information
marco-c committed Oct 22, 2024
1 parent f5a96f8 commit 84d035b
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 13 deletions.
7 changes: 2 additions & 5 deletions dom/geolocation/Geolocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -364,11 +364,8 @@ void Geolocation::ReallowWithSystemPermissionOrCancel(
geolocation::SystemGeolocationPermissionBehavior::GeckoWillPromptUser;


const auto kSpinnerNoButtonFlags = nsIPromptService::BUTTON_TITLE_IS_STRING *
nsIPromptService::BUTTON_POS_0 +
nsIPromptService::BUTTON_TITLE_IS_STRING *
nsIPromptService::BUTTON_POS_1 +
nsIPromptService::SHOW_SPINNER;
const auto kSpinnerNoButtonFlags =
nsIPromptService::BUTTON_NONE | nsIPromptService::SHOW_SPINNER;


const auto kCancelButtonFlags =
Expand Down
7 changes: 7 additions & 0 deletions netwerk/base/nsIPrompt.idl
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ interface nsIPrompt : nsISupports

const unsigned long SHOW_SPINNER = 1 << 27;


const unsigned long BUTTON_NONE_ENABLE_BIT = 1 << 28;

const unsigned long BUTTON_NONE =
BUTTON_NONE_ENABLE_BIT |
BUTTON_TITLE_IS_STRING * BUTTON_POS_0;

const unsigned long STD_OK_CANCEL_BUTTONS = (BUTTON_TITLE_OK * BUTTON_POS_0) +
(BUTTON_TITLE_CANCEL * BUTTON_POS_1);
const unsigned long STD_YES_NO_BUTTONS = (BUTTON_TITLE_YES * BUTTON_POS_0) +
Expand Down
5 changes: 5 additions & 0 deletions toolkit/components/prompts/src/CommonDialog.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ export class CommonDialog {
if (this.args.button3Label) {
numButtons++;
}
if (numButtons == 0 && !this.args.allowNoButtons) {
throw new Error(
"A dialog with no buttons requires the allowNoButtons argument"
);
}
this.numButtons = numButtons;
this.hasInputField = false;
this.iconClass = ["question-icon"];
Expand Down
30 changes: 28 additions & 2 deletions toolkit/components/prompts/src/Prompter.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,24 @@ var InternalPromptUtils = {
let defaultButtonNum = (flags & BUTTON_DEFAULT_MASK) >> 24;
let isDelayEnabled = flags & Ci.nsIPrompt.BUTTON_DELAY_ENABLE;

// Sanity check: If the flags indicate there should be no button0 then flags
// must equal BUTTON_NONE (notably, it must include BUTTON_NONE_ENABLE_BIT).
let allowNoButtons = flags == Ci.nsIPromptService.BUTTON_NONE;
const NO_BUTTON0 =
Ci.nsIPrompt.BUTTON_POS_0 * Ci.nsIPrompt.BUTTON_TITLE_IS_STRING;
if (!allowNoButtons && !button0 && (flags & NO_BUTTON0) == NO_BUTTON0) {
throw new Error(
`Request for modal prompt with no buttons requires flags to be ` +
`BUTTON_NONE. Got ${flags}`
);
}
if (allowNoButtons && (button0 || button1 || button2)) {
throw new Error(
`Request for modal prompt with no buttons requires button names to be ` +
`null. Got ${button0}, ${button1}, ${button2}`
);
}

// Flags can be used to select a specific pre-defined button label or
// a caller-supplied string (button0/button1/button2). If no flags are
// set for a button, then the button won't be shown.
Expand Down Expand Up @@ -732,6 +750,7 @@ var InternalPromptUtils = {
buttonLabels[2],
defaultButtonNum,
isDelayEnabled,
allowNoButtons,
];
},

Expand Down Expand Up @@ -1459,11 +1478,18 @@ class ModalPrompter {
...extraArgs,
};

let [label0, label1, label2, defaultButtonNum, isDelayEnabled] =
InternalPromptUtils.confirmExHelper(flags, button0, button1, button2);
let [
label0,
label1,
label2,
defaultButtonNum,
isDelayEnabled,
allowNoButtons,
] = InternalPromptUtils.confirmExHelper(flags, button0, button1, button2);

args.defaultButtonNum = defaultButtonNum;
args.enableDelay = isDelayEnabled;
args.allowNoButtons = allowNoButtons;

if (label0) {
args.button0Label = label0;
Expand Down
33 changes: 29 additions & 4 deletions toolkit/components/prompts/test/test_modal_prompts.html
Original file line number Diff line number Diff line change
Expand Up @@ -780,10 +780,7 @@

promptDone = handlePrompt(state, action);

// BUTTON_TITLE_IS_STRING with no title removes the button.
flags =
Ci.nsIPromptService.BUTTON_TITLE_IS_STRING * Ci.nsIPromptService.BUTTON_POS_1 +
Ci.nsIPromptService.BUTTON_TITLE_IS_STRING * Ci.nsIPromptService.BUTTON_POS_0;
flags = Ci.nsIPromptService.BUTTON_NONE;
promptArgs = [
"TestTitle",
"This is the confirmEx text.",
Expand All @@ -803,6 +800,34 @@

await promptDone;

// =====
info("Starting test: ConfirmEx (invalid request for no buttons)");
// BUTTON_TITLE_IS_STRING with no title removes the button. This is forbidden
// for button0 unless flags includes
// Ci.nsIPromptService.BUTTON_NONE_ENABLE_BIT.
flags =
Ci.nsIPromptService.BUTTON_TITLE_IS_STRING *
Ci.nsIPromptService.BUTTON_POS_0;

promptArgs = [
"TestTitle",
"This is the confirmEx text.",
flags,
null,
null,
null,
null,
util.useAsync ? false : {},
];

let gotException = false;
try {
await util.prompt("confirmEx", promptArgs);
} catch {
gotException = true;
}
ok(gotException, "Received exception for invalid button request");

// =====
info("Starting test: ConfirmEx (ok/cancel, ok)");
state = {
Expand Down
22 changes: 20 additions & 2 deletions toolkit/components/windowwatcher/nsIPromptService.idl
Original file line number Diff line number Diff line change
Expand Up @@ -268,15 +268,29 @@ interface nsIPromptService : nsISupports
const unsigned long SHOW_SPINNER = 1 << 27;




const unsigned long BUTTON_NONE_ENABLE_BIT = 1 << 28;






const unsigned long BUTTON_NONE =
BUTTON_NONE_ENABLE_BIT |
BUTTON_TITLE_IS_STRING * BUTTON_POS_0;

const unsigned long STD_OK_CANCEL_BUTTONS = (BUTTON_TITLE_OK * BUTTON_POS_0) +



const unsigned long STD_OK_CANCEL_BUTTONS = (BUTTON_TITLE_OK * BUTTON_POS_0) |
(BUTTON_TITLE_CANCEL * BUTTON_POS_1);




const unsigned long STD_YES_NO_BUTTONS = (BUTTON_TITLE_YES * BUTTON_POS_0) +
const unsigned long STD_YES_NO_BUTTONS = (BUTTON_TITLE_YES * BUTTON_POS_0) |
(BUTTON_TITLE_NO * BUTTON_POS_1);


Expand Down Expand Up @@ -325,6 +339,10 @@ interface nsIPromptService : nsISupports










Expand Down

0 comments on commit 84d035b

Please sign in to comment.