Bug 1925401: Require BUTTON_NONE in nsIPrompt for button-less dialogs…

… 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

dom/geolocation/Geolocation.cpp

@@ -362,13 +362,10 @@ void Geolocation::ReallowWithSystemPermissionOrCancel(
   bool geckoWillPrompt =
       GetLocationOSPermission() ==
       geolocation::SystemGeolocationPermissionBehavior::GeckoWillPromptUser;
-  // This combination of flags removes the default yes and no buttons and adds a
-  // spinner to the title.
-  const auto kSpinnerNoButtonFlags = nsIPromptService::BUTTON_TITLE_IS_STRING *
-                                         nsIPromptService::BUTTON_POS_0 +
-                                     nsIPromptService::BUTTON_TITLE_IS_STRING *
-                                         nsIPromptService::BUTTON_POS_1 +
-                                     nsIPromptService::SHOW_SPINNER;
+  // This combination of flags removes all buttons and adds a spinner to the
+  // title.
+  const auto kSpinnerNoButtonFlags =
+      nsIPromptService::BUTTON_NONE | nsIPromptService::SHOW_SPINNER;
   // This combination of flags indicates there is only one button labeled
   // "Cancel".
   const auto kCancelButtonFlags =

netwerk/base/nsIPrompt.idl

@@ -58,6 +58,13 @@ interface nsIPrompt : nsISupports
 
     const unsigned long SHOW_SPINNER               = 1 << 27;
 
+    // Part of BUTTON_NONE.  This is not a button flag.
+    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) +

toolkit/components/prompts/src/CommonDialog.sys.mjs

@@ -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"];

toolkit/components/prompts/src/Prompter.sys.mjs

@@ -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.
@@ -732,6 +750,7 @@ var InternalPromptUtils = {
       buttonLabels[2],
       defaultButtonNum,
       isDelayEnabled,
+      allowNoButtons,
     ];
   },
 
@@ -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;

toolkit/components/prompts/test/test_modal_prompts.html

@@ -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.",
@@ -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 = {

toolkit/components/windowwatcher/nsIPromptService.idl

@@ -267,16 +267,30 @@ interface nsIPromptService : nsISupports
    */
   const unsigned long SHOW_SPINNER               = 1 << 27;
 
+  // Part of BUTTON_NONE.  This is not a button flag.  It protects callers from
+  // unwittingly creating buttonless dialogs, since they must be manually
+  // dismissed by code.  See BUTTON_NONE.
+  const unsigned long BUTTON_NONE_ENABLE_BIT    = 1 << 28;
+
+  /**
+   * BUTTON_NONE indicates that the prompt should have no buttons.  The prompt
+   * must be dismissed in code, by calling prompt.close() with a "dialogclosing"
+   * event.  This flag should not be combined with other BUTTON_ flags.
+   */
+  const unsigned long BUTTON_NONE =
+      BUTTON_NONE_ENABLE_BIT |
+      BUTTON_TITLE_IS_STRING * BUTTON_POS_0;
+
   /**
    * Selects the standard set of OK/Cancel buttons.
    */
-  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);
 
   /**
    * Selects the standard set of Yes/No buttons.
    */
-  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);
 
   // Indicates whether a prompt should be shown in-content, on tab level or as a separate window
@@ -321,14 +335,18 @@ interface nsIPromptService : nsISupports
    * assign the title to a particular button.  If BUTTON_TITLE_IS_STRING is
    * used for a button, the string parameter for that button will be used.  If
    * the value for a button position is zero, the button will not be shown.
+   * Note that button 0 will throw an exception if hidden this way.  Clients
+   * must use BUTTON_NONE if they wish to hide all buttons.
    *
    * In general, aButtonFlags is constructed per the following example:
    *
-   *   aButtonFlags = (BUTTON_POS_0) * (BUTTON_TITLE_AAA) +
-   *                  (BUTTON_POS_1) * (BUTTON_TITLE_BBB) +
-   *                   BUTTON_POS_1_DEFAULT;
+   *   aButtonFlags = (BUTTON_POS_0) * (BUTTON_TITLE_AAA) |
+   *                  (BUTTON_POS_1) * (BUTTON_TITLE_BBB) |
+   *                  BUTTON_POS_1_DEFAULT;
    *
    * where "AAA" and "BBB" correspond to one of the button titles.
+   * Some older code uses '+' instead of '|' for constructing flags.  Please
+   * use '|' for this, as shown here.
    */
   int32_t confirmEx(in mozIDOMWindowProxy aParent,
                     in wstring aDialogTitle,