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

Issue 756675 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Feature

Blocking:
issue 740783



Sign in to add a comment

desktop-pwas: "Open link in new tab" and "Open link in new window" should not open app window

Project Member Reported by ortuno@chromium.org, Aug 18 2017

Issue description

These context menu options are explicit requests to open a new tab/window. Opening an app window would be unexpected.
 

Comment 1 by ortuno@chromium.org, Sep 13 2017

Components: UI>Browser>WebAppInstalls
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 30 2017

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

commit 3e7d447315decb4bd077ad41ede4bdeb180ecaa4
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Thu Nov 30 07:03:03 2017

desktop-pwas: Don't intercept navigations started from the context menu

Actions from the context menu are explicit, so intercepting navigations
from these actions would be unexpected.

Tests added for "Open link in new tab", for regular browser windows and
when in an app, and "Open link in new window" for regular browser
windows. There is no "Open link in new window" when in an app, so
there is no test for that option.

Bug:  756675 
Change-Id: I53ecbf9225d86a144db8e7b58b7743e09dbf8b77
Reviewed-on: https://chromium-review.googlesource.com/790210
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520467}
[modify] https://crrev.com/3e7d447315decb4bd077ad41ede4bdeb180ecaa4/chrome/browser/extensions/bookmark_app_navigation_throttle.cc
[modify] https://crrev.com/3e7d447315decb4bd077ad41ede4bdeb180ecaa4/chrome/browser/extensions/bookmark_app_navigation_throttle_browsertest.cc

Comment 3 by ortuno@chromium.org, Nov 30 2017

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 8 2018

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

commit 640873405ef191d307bac51759c0eda66485bc36
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Thu Feb 08 23:01:44 2018

content: Associate WindowOpenDisposition with the current navigation

It doesn't make sense to wire this information back to the content layer
so BrowserNavigator saves the disposition in a NavigationUIData that we
pass to the NavigationController.

Then NavigationHandleImpl retrieves this NavigationUIData and saves it.
Throttles that need the NavigationUIData can access it from there. See
https://crrev.com/c/907773 for an example.

The disposition is only relevant for main frames navigations so we don't
pass the NavigationUIData to NavigationController for subframe
navigations.

Bug:  756675 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Ie3fc81a772c8d0c1cd02ddaef0daf6272bda25c8
Reviewed-on: https://chromium-review.googlesource.com/633967
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535560}
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/chrome/browser/renderer_host/chrome_navigation_ui_data.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/chrome/browser/renderer_host/chrome_navigation_ui_data.h
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/chrome/browser/ui/browser_navigator_browsertest.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/form_submission_throttle_browsertest.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/navigation_controller_impl.h
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/navigation_controller_impl_unittest.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/navigation_handle_impl.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/navigation_handle_impl.h
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/navigation_handle_impl_unittest.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/navigation_request.h
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/navigator.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/navigator.h
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/navigator_impl.h
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/browser/frame_host/render_frame_host_manager_unittest.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/public/browser/navigation_controller.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/public/browser/navigation_controller.h
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/public/browser/navigation_handle.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/public/browser/navigation_handle.h
[add] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/public/test/test_navigation_ui_data.cc
[add] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/public/test/test_navigation_ui_data.h
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/content/test/BUILD.gn
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/extensions/browser/extension_navigation_ui_data.cc
[modify] https://crrev.com/640873405ef191d307bac51759c0eda66485bc36/extensions/browser/extension_navigation_ui_data.h

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 16 2018

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

commit f90df0d93bf7e350297cd05048d23c14e14b140d
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Fri Feb 16 06:59:07 2018

desktop-pwas: Use disposition to not capture Ctrl + Click

Changes BookmarkAppNavigationThrottle to retrieve the disposition
and open the app if the disposition is CURRENT_TAB, FOREGROUND_TAB, or
NEW_WINDOW. This means that navigations started with Ctrl + Click will
no longer result in an app window being opened.

Bug:  756675 
Change-Id: I4975c967554f1c380edeb2a49ed1c47dbc324afa
Reviewed-on: https://chromium-review.googlesource.com/907773
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537241}
[modify] https://crrev.com/f90df0d93bf7e350297cd05048d23c14e14b140d/chrome/browser/extensions/bookmark_app_navigation_throttle.cc
[modify] https://crrev.com/f90df0d93bf7e350297cd05048d23c14e14b140d/chrome/browser/extensions/bookmark_app_navigation_throttle.h
[modify] https://crrev.com/f90df0d93bf7e350297cd05048d23c14e14b140d/chrome/browser/extensions/bookmark_app_navigation_throttle_browsertest.cc
[modify] https://crrev.com/f90df0d93bf7e350297cd05048d23c14e14b140d/tools/metrics/histograms/enums.xml

Sign in to add a comment