Merge pull request #11613 from electron/safe-dialogs

Implement dialog (alert/confirm) blocking as a user switch after the first dialog

atom/browser/atom_javascript_dialog_manager.cc

@@ -10,6 +10,7 @@
 #include "atom/browser/api/atom_api_web_contents.h"
 #include "atom/browser/native_window.h"
 #include "atom/browser/ui/message_box.h"
+#include "atom/browser/web_contents_preferences.h"
 #include "base/bind.h"
 #include "base/strings/utf_string_conversions.h"
 #include "ui/gfx/image/image_skia.h"
@@ -18,6 +19,12 @@ using content::JavaScriptDialogType;
 
 namespace atom {
 
+namespace {
+
+constexpr int kUserWantsNoMoreDialogs = -1;
+
+}  // namespace
+
 AtomJavaScriptDialogManager::AtomJavaScriptDialogManager(
     api::WebContents* api_web_contents)
     : api_web_contents_(api_web_contents) {}
@@ -30,6 +37,11 @@ void AtomJavaScriptDialogManager::RunJavaScriptDialog(
     const base::string16& default_prompt_text,
     const DialogClosedCallback& callback,
     bool* did_suppress_message) {
+  const std::string origin = origin_url.GetOrigin().spec();
+  if (origin_counts_[origin] == kUserWantsNoMoreDialogs) {
+    return callback.Run(false, base::string16());
+  }
+
   if (dialog_type != JavaScriptDialogType::JAVASCRIPT_DIALOG_TYPE_ALERT &&
       dialog_type != JavaScriptDialogType::JAVASCRIPT_DIALOG_TYPE_CONFIRM) {
     callback.Run(false, base::string16());
@@ -41,12 +53,25 @@ void AtomJavaScriptDialogManager::RunJavaScriptDialog(
     buttons.push_back("Cancel");
   }
 
-  atom::ShowMessageBox(NativeWindow::FromWebContents(web_contents),
+  origin_counts_[origin]++;
-                       atom::MessageBoxType::MESSAGE_BOX_TYPE_NONE, buttons, -1,
+
-                       0, atom::MessageBoxOptions::MESSAGE_BOX_NONE, "",
+  std::string checkbox_string;
-                       base::UTF16ToUTF8(message_text), "", "", false,
+  if (origin_counts_[origin] > 1 &&
-                       gfx::ImageSkia(),
+      WebContentsPreferences::IsPreferenceEnabled("safeDialogs",
-                       base::Bind(&OnMessageBoxCallback, callback));
+                                                  web_contents)) {
+    if (!WebContentsPreferences::GetString("safeDialogsMessage",
+                                           &checkbox_string, web_contents)) {
+      checkbox_string = "Prevent this app from creating additional dialogs";
+    }
+  }
+  atom::ShowMessageBox(
+      NativeWindow::FromWebContents(web_contents),
+      atom::MessageBoxType::MESSAGE_BOX_TYPE_NONE, buttons, -1, 0,
+      atom::MessageBoxOptions::MESSAGE_BOX_NONE, "",
+      base::UTF16ToUTF8(message_text), "", checkbox_string,
+      false, gfx::ImageSkia(),
+      base::Bind(&AtomJavaScriptDialogManager::OnMessageBoxCallback,
+                 base::Unretained(this), callback, origin));
 }
 
 void AtomJavaScriptDialogManager::RunBeforeUnloadDialog(
@@ -63,11 +88,13 @@ void AtomJavaScriptDialogManager::CancelDialogs(
     bool reset_state) {
 }
 
-// static
 void AtomJavaScriptDialogManager::OnMessageBoxCallback(
     const DialogClosedCallback& callback,
+    const std::string& origin,
     int code,
     bool checkbox_checked) {
+  if (checkbox_checked)
+    origin_counts_[origin] = kUserWantsNoMoreDialogs;
   callback.Run(code == 0, base::string16());
 }
 

atom/browser/atom_javascript_dialog_manager.h

@@ -5,6 +5,7 @@
 #ifndef ATOM_BROWSER_ATOM_JAVASCRIPT_DIALOG_MANAGER_H_
 #define ATOM_BROWSER_ATOM_JAVASCRIPT_DIALOG_MANAGER_H_
 
+#include <map>
 #include <string>
 
 #include "content/public/browser/javascript_dialog_manager.h"
@@ -36,10 +37,13 @@ class AtomJavaScriptDialogManager : public content::JavaScriptDialogManager {
                      bool reset_state) override;
 
  private:
-  static void OnMessageBoxCallback(const DialogClosedCallback& callback,
+  void OnMessageBoxCallback(const DialogClosedCallback& callback,
+                            const std::string& origin,
                             int code,
                             bool checkbox_checked);
+
   api::WebContents* api_web_contents_;
+  std::map<std::string, int> origin_counts_;
 };
 
 }  // namespace atom

atom/browser/web_contents_preferences.cc

@@ -301,4 +301,13 @@ bool WebContentsPreferences::GetInteger(const std::string& attributeName,
   return false;
 }
 
+bool WebContentsPreferences::GetString(const std::string& attribute_name,
+                                       std::string* string_value,
+                                       content::WebContents* web_contents) {
+  WebContentsPreferences* self = FromWebContents(web_contents);
+  if (!self)
+    return false;
+  return self->web_preferences()->GetString(attribute_name, string_value);
+}
+
 }  // namespace atom

atom/browser/web_contents_preferences.h

@@ -40,6 +40,10 @@ class WebContentsPreferences
   static bool IsPreferenceEnabled(const std::string& attribute_name,
                                   content::WebContents* web_contents);
 
+  static bool GetString(const std::string& attribute_name,
+                        std::string* string_value,
+                        content::WebContents* web_contents);
+
   // Modify the WebPreferences according to |web_contents|'s preferences.
   static void OverrideWebkitPrefs(
       content::WebContents* web_contents, content::WebPreferences* prefs);

docs/api/browser-window.md

@@ -362,6 +362,12 @@ It creates a new `BrowserWindow` with native properties as set by the `options`.
     * `additionArguments` String[] (optional) - A list of strings that will be appended
       to `process.argv` in the renderer process of this app.  Useful for passing small
       bits of data down to renderer process preload scripts.
+    * `safeDialogs` Boolean (optional) - Whether to enable browser style
+      consecutive dialog protection. Default is `false`.
+    * `safeDialogsMessage` String (optional) - The message to display when
+      consecutive dialog protection is triggered. If not defined the default
+      message would be used, note that currently the default message is in
+      English and not localized.
 
 When setting minimum or maximum window size with `minWidth`/`maxWidth`/
 `minHeight`/`maxHeight`, it only constrains the users. It won't prevent you from

lib/browser/rpc-server.js

@@ -438,39 +438,6 @@ ipcMain.on('ELECTRON_BROWSER_SEND_TO', function (event, sendToAll, webContentsId
   }
 })
 
-// Implements window.alert(message, title)
-ipcMain.on('ELECTRON_BROWSER_WINDOW_ALERT', function (event, message, title) {
-  if (message == null) message = ''
-  if (title == null) title = ''
-
-  const dialogProperties = {
-    message: `${message}`,
-    title: `${title}`,
-    buttons: ['OK']
-  }
-  event.returnValue = event.sender.isOffscreen()
-    ? electron.dialog.showMessageBox(dialogProperties)
-    : electron.dialog.showMessageBox(
-      event.sender.getOwnerBrowserWindow(), dialogProperties)
-})
-
-// Implements window.confirm(message, title)
-ipcMain.on('ELECTRON_BROWSER_WINDOW_CONFIRM', function (event, message, title) {
-  if (message == null) message = ''
-  if (title == null) title = ''
-
-  const dialogProperties = {
-    message: `${message}`,
-    title: `${title}`,
-    buttons: ['OK', 'Cancel'],
-    cancelId: 1
-  }
-  event.returnValue = !(event.sender.isOffscreen()
-    ? electron.dialog.showMessageBox(dialogProperties)
-    : electron.dialog.showMessageBox(
-      event.sender.getOwnerBrowserWindow(), dialogProperties))
-})
-
 // Implements window.close()
 ipcMain.on('ELECTRON_BROWSER_WINDOW_CLOSE', function (event) {
   const window = event.sender.getOwnerBrowserWindow()

lib/renderer/window-setup.js

@@ -133,14 +133,6 @@ module.exports = (ipcRenderer, guestInstanceId, openerId, hiddenPage, usesNative
     }
   }
 
-  window.alert = function (message, title) {
-    ipcRenderer.sendSync('ELECTRON_BROWSER_WINDOW_ALERT', toString(message), toString(title))
-  }
-
-  window.confirm = function (message, title) {
-    return ipcRenderer.sendSync('ELECTRON_BROWSER_WINDOW_CONFIRM', toString(message), toString(title))
-  }
-
   // But we do not support prompt().
   window.prompt = function () {
     throw new Error('prompt() is and will not be supported.')

spec/chromium-spec.js

@@ -1088,10 +1088,6 @@ describe('chromium feature', () => {
       assert.throws(() => {
         window.alert({toString: null})
       }, /Cannot convert object to primitive value/)
-
-      assert.throws(() => {
-        window.alert('message', {toString: 3})
-      }, /Cannot convert object to primitive value/)
     })
   })
 
@@ -1100,10 +1096,6 @@ describe('chromium feature', () => {
       assert.throws(() => {
         window.confirm({toString: null}, 'title')
       }, /Cannot convert object to primitive value/)
-
-      assert.throws(() => {
-        window.confirm('message', {toString: 3})
-      }, /Cannot convert object to primitive value/)
     })
   })