Project: chromium Issues People Development process History Sign in
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 Project OldSpice Phase 1: Dismiss JavaScript dialogs on switch-away
Starred by 29 users Project Member Reported by a...@chromium.org, Jul 20 2016 Back to list
Status: Started
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Feature

Blocked on:
issue 682060



Sign in to add a comment
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
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
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
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
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
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
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
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
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

Project Member Comment 14 by bugdroid1@chromium.org, Nov 16
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?
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.
Labels: -Merge-Review-56 Merge-Approved-56
Approving for merge into M56 (build 2924)
Project Member Comment 18 by bugdroid1@chromium.org, Dec 1
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

Cc: durga.behera@chromium.org jochen@chromium.org
Issue 673166 has been merged into this issue.
Blockedon: 682060
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.
Issue 682855 has been merged into this issue.
Issue 699364 has been merged into this issue.
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.
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, Yesterday (26 hours ago)
Cc: rsesek@chromium.org
Issue 714459 has been merged into this issue.
Comment 28 by rsesek@chromium.org, Yesterday (26 hours ago)
Cc: -rsesek@chromium.org
Sign in to add a comment