New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 629964 link

Starred by 30 users
Status: Fixed
Owner:
a...@chromium.org
Closed: May 2017
Cc:
jochen@chromium.org
a...@chromium.org
hwi@chromium.org
█ hdodda@chromium.org
█ yiyaoliu@chromium.org
emilyschechter@chromium.org
tapted@chromium.org
█ ainslie@chromium.org
ellyjo...@chromium.org
pinkerton@chromium.org
█ shrike@chromium.org
█ durga.behera@chromium.org
█ f...@chromium.org
█ rpop@chromium.org
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Feature

Blocked on:
issue 682060
issue 716794
Blocking:
issue 439267
issue 629324


Show other hotlists

Hotlists containing this issue:
Interventions


Sign in to add a comment

Project OldSpice Phase 1: Dismiss JavaScript dialogs on switch-away

Project Member Reported by a...@chromium.org, Jul 20 2016 
Project OldSpice is an effort to tame the power of JavaScript dialogs. The design doc is at http://bit.ly/project-oldspice

This bug is to track the implementation of phase 1, as listed in the "Possible steps towards implementation" section, namely:

• Auto-dismissal (dismissing dialogs when switching away from the tabs displaying them) + ignoring background prompt + focus stealing with alert/confirm (though they would be auto-dismissed on switching out of that tab)
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 21 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/175a82c72429ec5799455abafe70694be6654ebe

commit 175a82c72429ec5799455abafe70694be6654ebe
Author: avi <avi@chromium.org>
Date: Thu Jul 21 18:47:28 2016

Rename the chrome/browser/ui/app_modal directory to be javascript_dialogs.

It will be the future home of the JavaScript dialog tab helper code, which will not be app-modal, so the name is no longer appropriate for use.

In addition, I'm the owner of the existing dialog code (components/app_modal) so I'm going to claim ownership of the new code.

BUG= 629964 
TEST=none

Review-Url: https://codereview.chromium.org/2169593003
Cr-Commit-Position: refs/heads/master@{#406920}

[modify] https://crrev.com/175a82c72429ec5799455abafe70694be6654ebe/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/175a82c72429ec5799455abafe70694be6654ebe/chrome/browser/ui/android/javascript_app_modal_dialog_android.cc
[delete] https://crrev.com/2cd2671b8999e8860727401ee1817d52f40c0d5c/chrome/browser/ui/app_modal/chrome_javascript_native_dialog_factory.h
[modify] https://crrev.com/175a82c72429ec5799455abafe70694be6654ebe/chrome/browser/ui/cocoa/javascript_app_modal_dialog_cocoa.mm
[add] https://crrev.com/175a82c72429ec5799455abafe70694be6654ebe/chrome/browser/ui/javascript_dialogs/OWNERS
[add] https://crrev.com/175a82c72429ec5799455abafe70694be6654ebe/chrome/browser/ui/javascript_dialogs/chrome_javascript_native_dialog_factory.h
[modify] https://crrev.com/175a82c72429ec5799455abafe70694be6654ebe/chrome/browser/ui/views/chrome_javascript_native_dialog_factory_views.cc
[modify] https://crrev.com/175a82c72429ec5799455abafe70694be6654ebe/chrome/chrome_browser_ui.gypi

Comment 3 by a...@chromium.org, Sep 21 2016

Cc: pinkerton@chromium.org ainslie@chromium.org tapted@chromium.org ellyjo...@chromium.org f...@chromium.org hwi@chromium.org emilyschechter@chromium.org a...@chromium.org shrike@chromium.org
 Issue 643355  has been merged into this issue.
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 12 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9

commit 5d3b869bc09c586f076e3d78d8b8966b75dcf6c9
Author: avi <avi@chromium.org>
Date: Wed Oct 12 22:00:46 2016

Simplify the JavaScriptDialogManager.

- This collapses two closely-related calls into just one.
- This allows the WebContents to specify no callbacks, which avoids calling back to it while it's being destructed. (This is a state that is not possible now, but will be possible soon.)

BUG= 629964 
TEST=existing tests

Review-Url: https://codereview.chromium.org/2412843002
Cr-Commit-Position: refs/heads/master@{#424875}

[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/android_webview/browser/aw_javascript_dialog_manager.cc
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/android_webview/browser/aw_javascript_dialog_manager.h
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/chrome/browser/extensions/content_script_apitest.cc
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.h
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/components/app_modal/app_modal_dialog.cc
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/components/app_modal/app_modal_dialog.h
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/components/app_modal/javascript_app_modal_dialog.cc
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/components/app_modal/javascript_app_modal_dialog.h
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/components/app_modal/javascript_dialog_manager.cc
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/components/app_modal/javascript_dialog_manager.h
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/content/browser/web_contents/web_contents_impl_browsertest.cc
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/content/public/browser/javascript_dialog_manager.h
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/content/shell/browser/shell_javascript_dialog_manager.cc
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/content/shell/browser/shell_javascript_dialog_manager.h
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/extensions/browser/guest_view/web_view/javascript_dialog_helper.cc
[modify] https://crrev.com/5d3b869bc09c586f076e3d78d8b8966b75dcf6c9/extensions/browser/guest_view/web_view/javascript_dialog_helper.h
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 19 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a1b55bb6e44c34caad1913f8ca4ca43b917eb29

commit 7a1b55bb6e44c34caad1913f8ca4ca43b917eb29
Author: avi <avi@chromium.org>
Date: Wed Oct 19 04:18:38 2016

Make JavaScript dialogs auto-dismiss on tab switch.

This is a Views-only implementation for now. This works on the Mac too, but is ugly; a real Mac version is to come.

Behind the feature flag "AutoDismissingDialogs".

BUG= 629964 

Review-Url: https://chromiumcodereview.appspot.com/2421943002
Cr-Commit-Position: refs/heads/master@{#426133}

[modify] https://crrev.com/7a1b55bb6e44c34caad1913f8ca4ca43b917eb29/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/7a1b55bb6e44c34caad1913f8ca4ca43b917eb29/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
[modify] https://crrev.com/7a1b55bb6e44c34caad1913f8ca4ca43b917eb29/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.h
[add] https://crrev.com/7a1b55bb6e44c34caad1913f8ca4ca43b917eb29/chrome/browser/ui/javascript_dialogs/javascript_dialog_views.cc
[add] https://crrev.com/7a1b55bb6e44c34caad1913f8ca4ca43b917eb29/chrome/browser/ui/javascript_dialogs/javascript_dialog_views.h
[modify] https://crrev.com/7a1b55bb6e44c34caad1913f8ca4ca43b917eb29/chrome/browser/ui/tab_helpers.cc
[modify] https://crrev.com/7a1b55bb6e44c34caad1913f8ca4ca43b917eb29/components/app_modal/javascript_dialog_manager.cc
[modify] https://crrev.com/7a1b55bb6e44c34caad1913f8ca4ca43b917eb29/components/app_modal/javascript_dialog_manager.h
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 26 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/713077b86adca2120992f5432afb2b58a1ff0909

commit 713077b86adca2120992f5432afb2b58a1ff0909
Author: avi <avi@chromium.org>
Date: Wed Oct 26 19:44:04 2016

When unblocking a WebContents, don't bring it to the front if it is not already.

BUG= 629964 ,  658772 

Review-Url: https://codereview.chromium.org/2443383002
Cr-Commit-Position: refs/heads/master@{#427776}

[modify] https://crrev.com/713077b86adca2120992f5432afb2b58a1ff0909/chrome/browser/ui/browser.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 28 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49faa64fae69901c98ee47c81a4806b14e0a0e86

commit 49faa64fae69901c98ee47c81a4806b14e0a0e86
Author: tapted <tapted@chromium.org>
Date: Fri Oct 28 08:18:43 2016

Split javascript_dialogs.cc into views and cocoa+views versions.

This allows them both to be compiled on Mac, which is needed for
mac_views_browser (since it doesn't link any Cocoa code).

Compile step regressed at go/macviewsbuilder/builds/16832.

BUG= 629964 ,  425229 

Review-Url: https://codereview.chromium.org/2458903002
Cr-Commit-Position: refs/heads/master@{#428316}

[modify] https://crrev.com/49faa64fae69901c98ee47c81a4806b14e0a0e86/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/49faa64fae69901c98ee47c81a4806b14e0a0e86/chrome/browser/ui/javascript_dialogs/javascript_dialog.cc
[add] https://crrev.com/49faa64fae69901c98ee47c81a4806b14e0a0e86/chrome/browser/ui/javascript_dialogs/javascript_dialog_mac.cc
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 31 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/25a3897a86b1a6b8932b92d1a1901789366d841c

commit 25a3897a86b1a6b8932b92d1a1901789366d841c
Author: avi <avi@chromium.org>
Date: Mon Oct 31 18:42:20 2016

Don't do a callback when suppressing a dialog.

Running the callback is already handled by the calling WebContents in that case.

BUG= 629964 

Review-Url: https://codereview.chromium.org/2468473002
Cr-Commit-Position: refs/heads/master@{#428758}

[modify] https://crrev.com/25a3897a86b1a6b8932b92d1a1901789366d841c/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 4 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bfbb4f279b04117cf70c6ae1c1baa5686d9baa5f

commit bfbb4f279b04117cf70c6ae1c1baa5686d9baa5f
Author: avi <avi@chromium.org>
Date: Fri Nov 04 19:56:44 2016

Prevent popunders with the new auto-dismissing dialogs.

BUG= 629964 

Review-Url: https://codereview.chromium.org/2455973006
Cr-Commit-Position: refs/heads/master@{#429986}

[modify] https://crrev.com/bfbb4f279b04117cf70c6ae1c1baa5686d9baa5f/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc
[modify] https://crrev.com/bfbb4f279b04117cf70c6ae1c1baa5686d9baa5f/chrome/browser/ui/javascript_dialogs/javascript_dialog.cc
[modify] https://crrev.com/bfbb4f279b04117cf70c6ae1c1baa5686d9baa5f/chrome/browser/ui/javascript_dialogs/javascript_dialog.h
[modify] https://crrev.com/bfbb4f279b04117cf70c6ae1c1baa5686d9baa5f/chrome/browser/ui/javascript_dialogs/javascript_dialog_cocoa.mm
[modify] https://crrev.com/bfbb4f279b04117cf70c6ae1c1baa5686d9baa5f/chrome/browser/ui/javascript_dialogs/javascript_dialog_mac.cc
[modify] https://crrev.com/bfbb4f279b04117cf70c6ae1c1baa5686d9baa5f/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
[modify] https://crrev.com/bfbb4f279b04117cf70c6ae1c1baa5686d9baa5f/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.h
[modify] https://crrev.com/bfbb4f279b04117cf70c6ae1c1baa5686d9baa5f/chrome/browser/ui/javascript_dialogs/javascript_dialog_views.cc
[modify] https://crrev.com/bfbb4f279b04117cf70c6ae1c1baa5686d9baa5f/chrome/common/chrome_features.cc
[modify] https://crrev.com/bfbb4f279b04117cf70c6ae1c1baa5686d9baa5f/chrome/common/chrome_features.h
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 11 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7484f1a2039a73e59ae812d6a15e21023c9827c7

commit 7484f1a2039a73e59ae812d6a15e21023c9827c7
Author: avi <avi@chromium.org>
Date: Fri Nov 11 01:59:36 2016

Log to the console when a dialog is suppressed or would be.

BUG= 629964 

Review-Url: https://codereview.chromium.org/2491163002
Cr-Commit-Position: refs/heads/master@{#431457}

[modify] https://crrev.com/7484f1a2039a73e59ae812d6a15e21023c9827c7/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2e37adb47f024c062c8249c8c7f1ce88d936b231

commit 2e37adb47f024c062c8249c8c7f1ce88d936b231
Author: avi <avi@chromium.org>
Date: Tue Nov 15 04:06:42 2016

Add UMA metrics to see the proportions of dialogs spawned by foremost pages.

BUG= 629964 

Review-Url: https://codereview.chromium.org/2502493002
Cr-Commit-Position: refs/heads/master@{#432093}

[modify] https://crrev.com/2e37adb47f024c062c8249c8c7f1ce88d936b231/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
[modify] https://crrev.com/2e37adb47f024c062c8249c8c7f1ce88d936b231/tools/metrics/histograms/histograms.xml

Comment 15 by a...@chromium.org, Nov 30 2016

Labels: Merge-Request-56
I just committed https://codereview.chromium.org/2535043002/ about metrics; can I get a merge to 56 for metrics about dialog closure?

Comment 16 by dimu@chromium.org, Nov 30 2016

Labels: -Merge-Request-56 Merge-Review-56 Hotlist-Merge-Review
[Automated comment] Commit may have occurred before M56 branch point (11/17/2016), needs manual review.

Comment 17 by bustamante@google.com, Dec 1 2016

Labels: -Merge-Review-56 Merge-Approved-56
Approving for merge into M56 (build 2924)
Project Member

Comment 18 by bugdroid1@chromium.org, Dec 1 2016

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/511f20e8743d2d5ef2b9ab812fb37428a9c3669c

commit 511f20e8743d2d5ef2b9ab812fb37428a9c3669c
Author: Avi Drissman <avi@chromium.org>
Date: Thu Dec 01 16:11:34 2016

Add UMA metrics to measure the dismissal cause of dialogs.

BUG= 629964 

Review-Url: https://codereview.chromium.org/2535043002
Cr-Commit-Position: refs/heads/master@{#435016}
(cherry picked from commit 70f9ea001b21e3ad255fe0d2bb7822d8e52789b7)

Review URL: https://codereview.chromium.org/2543793003 .

Cr-Commit-Position: refs/branch-heads/2924@{#247}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/511f20e8743d2d5ef2b9ab812fb37428a9c3669c/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
[modify] https://crrev.com/511f20e8743d2d5ef2b9ab812fb37428a9c3669c/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.h
[modify] https://crrev.com/511f20e8743d2d5ef2b9ab812fb37428a9c3669c/tools/metrics/histograms/histograms.xml

Comment 19 by a...@chromium.org, Jan 3 2017

Cc: durga.behera@chromium.org jochen@chromium.org
 Issue 673166  has been merged into this issue.

Comment 20 by a...@chromium.org, Jan 18 2017

Blockedon: 682060

Comment 22 by miqrogro...@gmail.com, Jan 20 2017 

It looks like some site are already trying to adapt to this by throwing the "Do you want to leave this site... changes will not be saved" dialog in combintation with various alert windows and autoplay audio.

Please make sure none of these features can prevent any future attempt to close a tab or window.

Comment 23 by a...@chromium.org, Jan 31 2017 

 Issue 682855  has been merged into this issue.

Comment 24 by a...@chromium.org, Mar 8 2017 

Issue 699364 has been merged into this issue.

Comment 25 by tobias...@gmail.com, Mar 28 2017 

A better alternative to the proposed auto-dismissal would be to make the dialogs modal to the tab, instead of modal to the window.  Firefox already does it this way.

Comment 26 by a...@chromium.org, Mar 29 2017 

Please read the design doc at http://bit.ly/project-oldspice . Firefox's approach is something we do not want to do.

Comment 27 by a...@chromium.org, Apr 24 2017

Cc: rsesek@chromium.org
 Issue 714459  has been merged into this issue.

Comment 28 by rsesek@chromium.org, Apr 24 2017

Cc: -rsesek@chromium.org

Comment 29 by a...@chromium.org, Apr 29 2017

Blockedon: 716794
Project Member

Comment 31 by bugdroid1@chromium.org, May 4 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69b6c44ab3317745f442a91b24b89e4e2f6ca20b

commit 69b6c44ab3317745f442a91b24b89e4e2f6ca20b
Author: avi <avi@chromium.org>
Date: Thu May 04 16:43:54 2017

Enable AutoDismissingDialogs by default.

This allows them to be on for the continuous official Win builders.

BUG= 629964 ,717947

Review-Url: https://codereview.chromium.org/2860093002
Cr-Commit-Position: refs/heads/master@{#469360}

[modify] https://crrev.com/69b6c44ab3317745f442a91b24b89e4e2f6ca20b/chrome/common/chrome_features.cc
[modify] https://crrev.com/69b6c44ab3317745f442a91b24b89e4e2f6ca20b/testing/variations/fieldtrial_testing_config.json

Comment 33 by a...@chromium.org, May 31 2017

Status: Fixed (was: Started)

Comment 34 by a...@chromium.org, May 31 2017 

Attaching a personal text document tracking CLs involved.
dialog change list.txt
1.9 KB View Download

Comment 35 by naftoli...@gmail.com, Jun 2 2017 

What's the point of such a breaking change? Can't all the problems be solved simply by making modals non-native and page-modal?

Comment 36 by sdy@chromium.org, Jun 6 2017 

Re. #35: See the design doc. In some cases, multiple pages share a JavaScript environment. When one is showing a blocking dialog, all of the others would be frozen.

Comment 37 by bran...@promotionsonly.com.au, Aug 16 2017 

so prompt() gives me this warning:
A window.prompt() dialog generated by this page was suppressed because this page is not the active tab of the front window. Please make sure your dialogs are triggered by user interactions to avoid this situation. https://www.chromestatus.com/feature/5637107137642496

I am currently on the active tab attempting to manually call prompt() like I did a few days ago when the above warning was returned
although I can use alert() and be taken to whatever inactive tab called it prompt seems to not work on neither the active tab or an inactive tab

Comment 38 by a...@chromium.org, Aug 16 2017 

Is it the active tab of the front-most window? If so, please file a new bug with information to reproduce the issue.

Prompt no longer activates tabs.

Comment 39 by patricia...@chromium.org, Sep 19 2017

Cc: hdodda@chromium.org
 Issue 761204  has been merged into this issue.

Comment 40 Deleted

Comment 41 by mig...@zappysoftware.com, Oct 18 2017 

Well this  wont allow to write on the devtools console, prompt("test") when the console is in window mode, and not docked to the current tab..

Issuing the warn A window.prompt() dialog generated by this page was suppressed because this page is not the active tab of the front window. Please make sure your dialogs are triggered by user interactions to avoid this situation. https://www.chromestatus.com/feature/5637107137642496 !

Comment 42 by a...@chromium.org, Oct 18 2017 

If you want to write to the devtools console, you should do so with "console.log".

Please see https://developers.google.com/web/updates/2017/03/dialogs-policy for the Chromium dialog policy.

Comment 43 by phistuck@gmail.com, Oct 18 2017 

#42 - the policy should exclude the scenario discussed in comment 41. Using the Developer Tools should be considered as the inspected page having focus, for dialog purposes. This is a bug.

Comment 44 by mig...@zappysoftware.com, Oct 24 2017 

I agree with #43 , this is a sort of bug, the developer tools is in focus of the window it related too.  Concerning #42 using console.log is not the samething, what if i want to test some console logic using the prompt.. console wont allow to that.

Comment 45 by rbyers@chromium.org, Jun 5 2018

Blocking: 439267
►

Sign in to add a comment