Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4532)

Unified Diff: chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc

Issue 2491163002: Log to the console when a dialog is suppressed or would be. (Closed)
Patch Set: phrasing! Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
diff --git a/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc b/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
index dd52e788c6c634380fed2c5ca9a9966a4076e6e1..88cde64276648e60760ca1d07abb0b12a125bcd8 100644
--- a/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
+++ b/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
@@ -16,6 +16,7 @@
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/common/chrome_features.h"
#include "components/app_modal/javascript_dialog_manager.h"
+#include "content/public/browser/render_frame_host.h"
DEFINE_WEB_CONTENTS_USER_DATA_KEY(JavaScriptDialogTabHelper);
@@ -80,13 +81,26 @@ void JavaScriptDialogTabHelper::RunJavaScriptDialog(
content::WebContents* parent_web_contents =
WebContentsObserver::web_contents();
- if (!IsWebContentsForemost(parent_web_contents) &&
- message_type == content::JAVASCRIPT_MESSAGE_TYPE_PROMPT) {
- // Don't allow "prompt" dialogs to steal the user's focus. TODO(avi):
- // Eventually, for subsequent phases of http://bit.ly/project-oldspice,
- // turn off focus stealing for other dialog types.
- *did_suppress_message = true;
- return;
+ if (!IsWebContentsForemost(parent_web_contents)) {
+ if (message_type == content::JAVASCRIPT_MESSAGE_TYPE_PROMPT) {
+ // Don't allow "prompt" dialogs to steal the user's focus. TODO(avi):
+ // Eventually, for subsequent phases of http://bit.ly/project-oldspice,
+ // turn off focus stealing for other dialog types.
+ *did_suppress_message = true;
+ alerting_web_contents->GetMainFrame()->AddMessageToConsole(
+ content::CONSOLE_MESSAGE_LEVEL_WARNING,
+ "A window.prompt() dialog generated by this page was suppressed "
+ "because this page is not the active tab of the front window. "
+ "Please do not use dialogs to force browser windows to the front.");
Rick Byers 2016/11/10 04:18:39 Nit: add chromestatus.com feature link for now (we
Rick Byers 2016/11/10 04:18:39 Is there some way for developers to actually do wh
Avi (use Gerrit) 2016/11/10 21:04:30 What they should do is not use prompt() untriggere
Avi (use Gerrit) 2016/11/10 21:04:30 Done.
Rick Byers 2016/11/11 00:43:33 Sounds good - thanks!
+ return;
+ }
+ // For now, let them off the hook with a warning.
+ alerting_web_contents->GetMainFrame()->AddMessageToConsole(
+ content::CONSOLE_MESSAGE_LEVEL_WARNING,
+ "A JavaScript dialog was generated by this page while this page was "
+ "not the active tab of the front window. Such dialogs will soon be "
+ "suppressed. Please do not use dialogs to force browser windows to "
+ "the front.");
Rick Byers 2016/11/10 04:18:39 Do you have UMA data on how often this occurs? We
Avi (use Gerrit) 2016/11/10 21:04:30 No, I don't, unfortunately. Do you want to not wa
Rick Byers 2016/11/11 00:43:33 I guess I'd rather discuss this one separately - i
}
if (dialog_) {
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698