desktop-pwas: Keep out of scope navigation in the same window with CCT UI |
||||||||||||
Issue descriptionChrome Version: 71 OS: All Desktop (Chrome OS, Windows, Linux, and macOS once we get PWAs there). Today, when a site navigates out-of-scope, we cancel the navigation and start a new one in a tab. Related spec issue: https://github.com/w3c/manifest/issues/646 What steps will reproduce the problem? (1) Go to https://killer-marmot.appspot.com/web/ (2) Install this app and open the installed app window. (3) Click "Fork me on GitHub". What is the expected result? The PWA app window navigates to GitHub, but because this is out of the app manifest's scope, a pseudo-location bar or "Chrome Custom Tab" (as it's called in Android) UI shows at the top of the window, indicating the origin of GitHub. Note: If you install a non-PWA (Bookmark App, using Create Shortcut), you actually do get the desired behaviour, but the UI is ugly and needs a rework. What happens instead? GitHub opens in a new browser tab. Bonus problem: From the same app: (3) Click "Post!" under "POST-and-redirect". After 3 seconds, it redirects back automatically. The expected behaviour is that the out-of-scope site https://redirectonpost.appspot.com appears in the same window in a CCT, then redirects back to Killer Marmot, hiding the CCT UI. The observed behaviour is that the out-of-scope site https://redirectonpost.appspot.com appears in a new browser tab, and then redirects back to Killer Marmot, resulting in a duplicate Killer Marmot page in a browser tab. This is typical of off-site authentication flows. Note: The current behaviour is actually mandated by the Manifest spec. This is a problem. We are working on a (breaking) change to the spec, at https://github.com/w3c/manifest/issues/646. In the mean time, Chrome will be intervening here because the broken auth flow is unacceptable, and we've already intervened on Android. List of problems with the current behaviour: 1. Sites that expected to be navigated away stay open in a weird state e.g. "Loading..." 2. Sometimes the popup blocker gets triggered and stops the new tab from being created. We can't disable the popup blocker because sites could abuse this and keep opening tabs. 3. Login flows get broken since the login flow gets moved to a tab. 4. Implementation-wise, there is no mechanism to redirect the navigation to a tab. Currently we cancel the navigation and try to replicate the original navigation in a tab. In the past, we've accidentally missed things when trying to replicate the navigation which has caused login and checkout flows to break. Because of these reasons, we should keep navigations in the original context but show some UI to indicate that the site has navigated off scope.
,
Jun 18 2018
,
Jul 9
,
Jul 9
,
Aug 16
,
Sep 13
Issue 883564 has been merged into this issue.
,
Sep 13
,
Sep 13
,
Sep 13
,
Sep 21
,
Sep 25
,
Oct 8
,
Oct 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ee7a7bc6174f740cf9c0c58438f637ccc6c62d37 commit ee7a7bc6174f740cf9c0c58438f637ccc6c62d37 Author: Jay Harris <harrisjay@chromium.org> Date: Tue Oct 09 01:47:49 2018 Adds a flag to open external links inside desktop PWAs The flag `desktop-pwas-stay-in-window` controls whether out of scope links open in an external browser. When disabled (the default) behavior remains the same as before this change (out of scope links open in the browser). When enabled, out of scope links will open in the PWA window and display the location bar (as we do already, in bookmark apps). This change has been made because the current flow (kicking the user to the browser) breaks authentication flows on some websites. Bug: 853593 Change-Id: Id38dbd8437651133b2b5be5f05260b342066d0ce Reviewed-on: https://chromium-review.googlesource.com/c/1242727 Commit-Queue: Jay Harris <harrisjay@chromium.org> Reviewed-by: Matt Giuca <mgiuca@chromium.org> Reviewed-by: Alan Cutter <alancutter@chromium.org> Cr-Commit-Position: refs/heads/master@{#597772} [modify] https://crrev.com/ee7a7bc6174f740cf9c0c58438f637ccc6c62d37/chrome/browser/about_flags.cc [modify] https://crrev.com/ee7a7bc6174f740cf9c0c58438f637ccc6c62d37/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/ee7a7bc6174f740cf9c0c58438f637ccc6c62d37/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/ee7a7bc6174f740cf9c0c58438f637ccc6c62d37/chrome/browser/flag_descriptions.h [modify] https://crrev.com/ee7a7bc6174f740cf9c0c58438f637ccc6c62d37/chrome/browser/ui/extensions/hosted_app_browsertest.cc [modify] https://crrev.com/ee7a7bc6174f740cf9c0c58438f637ccc6c62d37/chrome/common/chrome_features.cc [modify] https://crrev.com/ee7a7bc6174f740cf9c0c58438f637ccc6c62d37/chrome/common/chrome_features.h [modify] https://crrev.com/ee7a7bc6174f740cf9c0c58438f637ccc6c62d37/tools/metrics/histograms/enums.xml
,
Oct 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a755e8f5209099ee6e60ee1e75cf83966e7f867e commit a755e8f5209099ee6e60ee1e75cf83966e7f867e Author: Jay Harris <harrisjay@chromium.org> Date: Thu Oct 11 00:51:58 2018 Enables feature desktop-pwas-stay-in-window by default Bug: 853593 Change-Id: I7fba430843c19e36cc75b0b51e4c3c0c5f607cd2 Reviewed-on: https://chromium-review.googlesource.com/c/1272635 Commit-Queue: Jay Harris <harrisjay@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Reviewed-by: Ben Wells <benwells@chromium.org> Cr-Commit-Position: refs/heads/master@{#598603} [modify] https://crrev.com/a755e8f5209099ee6e60ee1e75cf83966e7f867e/chrome/browser/extensions/bookmark_app_navigation_throttle_browsertest.cc [modify] https://crrev.com/a755e8f5209099ee6e60ee1e75cf83966e7f867e/chrome/browser/ui/views/intent_picker_bubble_view_browsertest.cc [modify] https://crrev.com/a755e8f5209099ee6e60ee1e75cf83966e7f867e/chrome/common/chrome_features.cc
,
Oct 17
i have a video from a navigation flow described by the owner of this topic. in chrome 69 it worked, for some reasons in the latest version of chrome (70) not anymore.
,
Oct 17
additional info: it works in "Google Chrome Canary" as expected. the redirection happens in the application window. on chrome-beta and on the released version 70 it happens like in the video shown.
,
Oct 18
@sergej.sachs: Thanks for reporting this. I think I can explain why it's working on 69 and 71 (Canary) but not 70. - In 69, we had not yet rolled out PWAs on Windows. So when you use "Create Shortcut" you aren't installing a PWA, and thus the login works inside the app window. - In 70, you are installing a PWA (not creating a shortcut). So when you use "Install app", you're installing a PWA, and the login takes place in a browser tab. We've found that this is broken in some cases. - In 71, we've fixed it so that login flows from PWAs take place in the app window, just like for a shortcut. So in some cases, this will be working properly in 69 and 71, but broken in 70. Aside from waiting until 71 comes out, a workaround may be: instead of choosing "Install" from the menu, go to "More tools" -> "Create shortcut". That *may* get you back to the old behaviour. I would be interested to hear if that works.
,
Oct 18
@mgiuca thx for answer, good to hear it will be fixed in 71. The strange thing is, if I use "Create shortcut" the app starts directly in chrome, does not open a application window at all. But i will wait for the 71 release.
,
Oct 18
@mgiuca is there a possible release date for chrome 71?
,
Oct 19
#18: If you go to chrome://apps, right-click the app icon, and tick "Open in window", it should launch in its own window. (Though I can't guarantee it will fix the navigation flow.) #19: Planning to release Chrome 71 in early December. (Usually there is a 6-week gap between each release.)
,
Oct 19
,
Oct 19
#20 thx for the information, i will wait for the next release. not sure if this problem occurs in chrome mobile. because chrome mobile is on version 69 but supports already pwa installations right? had no time to test it for now.
,
Oct 19
Thanks for bearing with us here. :) Chrome on Android has had the fix here for a while - I believe since v67.
,
Oct 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/96a0c5e6d815f70ef9c63b6b05131586e858d5b3 commit 96a0c5e6d815f70ef9c63b6b05131586e858d5b3 Author: Jay Harris <harrisjay@chromium.org> Date: Tue Oct 30 03:01:15 2018 Makes the LocationIconView more self contained. Dependencies on the LocationBarView have been abstracted into a delegate, so as to make the LocationIconView reusable. This is in preparation for introducing a CustomTabBarView for Desktop PWAs, which will have its own LocationIconView. Bug: 853593 Change-Id: I5817041e532cd8db6065f2a818e6b203a91fb012 Reviewed-on: https://chromium-review.googlesource.com/c/1248067 Commit-Queue: Jay Harris <harrisjay@chromium.org> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Cr-Commit-Position: refs/heads/master@{#603752} [modify] https://crrev.com/96a0c5e6d815f70ef9c63b6b05131586e858d5b3/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/96a0c5e6d815f70ef9c63b6b05131586e858d5b3/chrome/browser/ui/views/location_bar/location_bar_view.h [modify] https://crrev.com/96a0c5e6d815f70ef9c63b6b05131586e858d5b3/chrome/browser/ui/views/location_bar/location_bar_view_browsertest.cc [modify] https://crrev.com/96a0c5e6d815f70ef9c63b6b05131586e858d5b3/chrome/browser/ui/views/location_bar/location_icon_view.cc [modify] https://crrev.com/96a0c5e6d815f70ef9c63b6b05131586e858d5b3/chrome/browser/ui/views/location_bar/location_icon_view.h [modify] https://crrev.com/96a0c5e6d815f70ef9c63b6b05131586e858d5b3/chrome/browser/ui/views/location_bar/selected_keyword_view.cc
,
Nov 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/52511a00b965c038c863668b378637bbe4dc2494 commit 52511a00b965c038c863668b378637bbe4dc2494 Author: Jay Harris <harrisjay@chromium.org> Date: Fri Nov 02 01:00:28 2018 Adds flag for using custom tab UI. This flag will control whether or not the new custom tab bar UI is used. Bug: 853593 Change-Id: Ie8ab91d10fc6543ab560a50795dd5e26b8e31146 Reviewed-on: https://chromium-review.googlesource.com/c/1312135 Commit-Queue: Jay Harris <harrisjay@chromium.org> Reviewed-by: Matt Giuca <mgiuca@chromium.org> Cr-Commit-Position: refs/heads/master@{#604787} [modify] https://crrev.com/52511a00b965c038c863668b378637bbe4dc2494/chrome/browser/about_flags.cc [modify] https://crrev.com/52511a00b965c038c863668b378637bbe4dc2494/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/52511a00b965c038c863668b378637bbe4dc2494/chrome/browser/flag_descriptions.h [modify] https://crrev.com/52511a00b965c038c863668b378637bbe4dc2494/chrome/common/chrome_features.cc [modify] https://crrev.com/52511a00b965c038c863668b378637bbe4dc2494/chrome/common/chrome_features.h [modify] https://crrev.com/52511a00b965c038c863668b378637bbe4dc2494/tools/metrics/histograms/enums.xml
,
Nov 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/afecf795d7b906dc99bca36cb65b6442ab493713 commit afecf795d7b906dc99bca36cb65b6442ab493713 Author: Jay Harris <harrisjay@chromium.org> Date: Fri Nov 09 04:41:46 2018 Displays the app title instead of page title when off scope. Previously, we would always display the current page's title in the window title. This updates the logic so we display the app name when off scope (the page title will be displayed in the custom tab bar). Bug: 853593 Change-Id: Id24d3104386a5334c331599480f0b1ce269ba9e1 Reviewed-on: https://chromium-review.googlesource.com/c/1319214 Reviewed-by: Matt Giuca <mgiuca@chromium.org> Commit-Queue: Jay Harris <harrisjay@chromium.org> Cr-Commit-Position: refs/heads/master@{#606736} [modify] https://crrev.com/afecf795d7b906dc99bca36cb65b6442ab493713/chrome/browser/ui/extensions/hosted_app_browser_controller.cc [modify] https://crrev.com/afecf795d7b906dc99bca36cb65b6442ab493713/chrome/browser/ui/extensions/hosted_app_browsertest.cc
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1c9a2691f0e3ad2814ee3549abbf9decbec15161 commit 1c9a2691f0e3ad2814ee3549abbf9decbec15161 Author: Jay Harris <harrisjay@chromium.org> Date: Thu Nov 15 05:43:58 2018 Prepares for addition of CustomTabBar. The CustomTabBar will be added in a future CL. * Lifts LocationBar animation into Toolbar, so the same animation can be reused on the CustomTabBar without duplication. * Renames ShouldShowLocationBar to ShouldShowToolbar * Renames UpdateLocationBarVisibility to UpdateToolbarVisibility Bug: 853593 Change-Id: Ib303275f7d560cb67cc2e30767a7dbf2783da497 Reviewed-on: https://chromium-review.googlesource.com/c/1328081 Commit-Queue: Jay Harris <harrisjay@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: Matt Giuca <mgiuca@chromium.org> Cr-Commit-Position: refs/heads/master@{#608274} [modify] https://crrev.com/1c9a2691f0e3ad2814ee3549abbf9decbec15161/chrome/browser/ui/browser.cc [modify] https://crrev.com/1c9a2691f0e3ad2814ee3549abbf9decbec15161/chrome/browser/ui/browser_window.h [modify] https://crrev.com/1c9a2691f0e3ad2814ee3549abbf9decbec15161/chrome/browser/ui/extensions/hosted_app_browser_controller.cc [modify] https://crrev.com/1c9a2691f0e3ad2814ee3549abbf9decbec15161/chrome/browser/ui/extensions/hosted_app_browser_controller.h [modify] https://crrev.com/1c9a2691f0e3ad2814ee3549abbf9decbec15161/chrome/browser/ui/extensions/hosted_app_browsertest.cc [modify] https://crrev.com/1c9a2691f0e3ad2814ee3549abbf9decbec15161/chrome/browser/ui/location_bar/location_bar.h [modify] https://crrev.com/1c9a2691f0e3ad2814ee3549abbf9decbec15161/chrome/browser/ui/views/frame/browser_view.cc [modify] https://crrev.com/1c9a2691f0e3ad2814ee3549abbf9decbec15161/chrome/browser/ui/views/frame/browser_view.h [modify] https://crrev.com/1c9a2691f0e3ad2814ee3549abbf9decbec15161/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/1c9a2691f0e3ad2814ee3549abbf9decbec15161/chrome/browser/ui/views/location_bar/location_bar_view.h [modify] https://crrev.com/1c9a2691f0e3ad2814ee3549abbf9decbec15161/chrome/browser/ui/views/toolbar/toolbar_view.cc [modify] https://crrev.com/1c9a2691f0e3ad2814ee3549abbf9decbec15161/chrome/browser/ui/views/toolbar/toolbar_view.h [modify] https://crrev.com/1c9a2691f0e3ad2814ee3549abbf9decbec15161/chrome/test/base/test_browser_window.h
,
Nov 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3dc4c27ed6af3e7ea3fbd11522e5cb140ddc3455 commit 3dc4c27ed6af3e7ea3fbd11522e5cb140ddc3455 Author: Jay Harris <harrisjay@chromium.org> Date: Wed Nov 28 22:18:52 2018 Fixes a bug in the LocationIconView InkDrop This stops the LocationIconView InkDrop from getting reset if LocationIconView::Update is called while the InkDrop is animating. Note: This issue doesn't seem to affect the LocationBarView, only the CustomTabBar being introduced in CL: 1328084 Bug: 853593 Change-Id: I3ab4045c7a8d0db88a069f1d35d9763c9c8594f1 Reviewed-on: https://chromium-review.googlesource.com/c/1338579 Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Commit-Queue: Jay Harris <harrisjay@chromium.org> Cr-Commit-Position: refs/heads/master@{#611893} [modify] https://crrev.com/3dc4c27ed6af3e7ea3fbd11522e5cb140ddc3455/chrome/browser/ui/views/location_bar/location_icon_view.cc [modify] https://crrev.com/3dc4c27ed6af3e7ea3fbd11522e5cb140ddc3455/chrome/browser/ui/views/location_bar/location_icon_view.h [modify] https://crrev.com/3dc4c27ed6af3e7ea3fbd11522e5cb140ddc3455/chrome/browser/ui/views/location_bar/location_icon_view_unittest.cc
,
Dec 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fb43687746522d4fc167d268725007c0761f57e1 commit fb43687746522d4fc167d268725007c0761f57e1 Author: Jay Harris <harrisjay@chromium.org> Date: Mon Dec 03 23:40:31 2018 Add a custom tab bar to use for desktop PWAs. Bug: 853593 Change-Id: I4618ca91c53a18ed7a56cb8dce0416d25318ab41 Reviewed-on: https://chromium-review.googlesource.com/c/1328084 Commit-Queue: Jay Harris <harrisjay@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: Matt Giuca <mgiuca@chromium.org> Cr-Commit-Position: refs/heads/master@{#613322} [modify] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/BUILD.gn [modify] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/browser.cc [modify] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/browser.h [modify] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/bubble_anchor_util.h [modify] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/extensions/hosted_app_browsertest.cc [modify] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/omnibox/omnibox_theme.cc [modify] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/omnibox/omnibox_theme.h [modify] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/views/bubble_anchor_util_views.cc [add] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/views/location_bar/custom_tab_bar_view.cc [add] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/views/location_bar/custom_tab_bar_view.h [add] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/views/location_bar/custom_tab_bar_view_browsertest.cc [modify] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/views/tabs/tab.cc [modify] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/views/toolbar/toolbar_view.cc [modify] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/browser/ui/views/toolbar/toolbar_view.h [modify] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/common/chrome_features.cc [modify] https://crrev.com/fb43687746522d4fc167d268725007c0761f57e1/chrome/test/BUILD.gn
,
Dec 10
Comparison of enabling and disabling AutoColorReadability on LocationIconView->label
,
Dec 10
You probably want to use (and upstream to the base class) GetReadableFrameForegroundColor() to match the colour of text in the title bar.
,
Dec 10
Is there a plan to add some sort of X or back gesture to "close" the CCT (i.e. navigate back to the first in-scope URL in the history list?). We do this on Android.
,
Dec 11
alancutter@ that's what I'm doing. dominickn@ there is, but not with this changeset :)
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a4175d119c87220ce17df68adc78f58b0c96374 commit 8a4175d119c87220ce17df68adc78f58b0c96374 Author: Jay Harris <harrisjay@chromium.org> Date: Wed Dec 12 04:06:56 2018 Use app theme color for custom tab bar. Bug: 853593 Change-Id: I4c3a8b4e21b283cce61579973c94236dfc057904 Reviewed-on: https://chromium-review.googlesource.com/c/1318771 Commit-Queue: Jay Harris <harrisjay@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#615811} [modify] https://crrev.com/8a4175d119c87220ce17df68adc78f58b0c96374/chrome/browser/ui/views/frame/browser_non_client_frame_view.h [modify] https://crrev.com/8a4175d119c87220ce17df68adc78f58b0c96374/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc [modify] https://crrev.com/8a4175d119c87220ce17df68adc78f58b0c96374/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h [modify] https://crrev.com/8a4175d119c87220ce17df68adc78f58b0c96374/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.h [modify] https://crrev.com/8a4175d119c87220ce17df68adc78f58b0c96374/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.mm [modify] https://crrev.com/8a4175d119c87220ce17df68adc78f58b0c96374/chrome/browser/ui/views/frame/glass_browser_frame_view.cc [modify] https://crrev.com/8a4175d119c87220ce17df68adc78f58b0c96374/chrome/browser/ui/views/frame/glass_browser_frame_view.h [modify] https://crrev.com/8a4175d119c87220ce17df68adc78f58b0c96374/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc [modify] https://crrev.com/8a4175d119c87220ce17df68adc78f58b0c96374/chrome/browser/ui/views/frame/opaque_browser_frame_view.h [modify] https://crrev.com/8a4175d119c87220ce17df68adc78f58b0c96374/chrome/browser/ui/views/location_bar/custom_tab_bar_view.cc [modify] https://crrev.com/8a4175d119c87220ce17df68adc78f58b0c96374/chrome/browser/ui/views/location_bar/custom_tab_bar_view.h [modify] https://crrev.com/8a4175d119c87220ce17df68adc78f58b0c96374/chrome/browser/ui/views/location_bar/location_icon_view.cc [modify] https://crrev.com/8a4175d119c87220ce17df68adc78f58b0c96374/chrome/browser/ui/views/toolbar/toolbar_view.cc
,
Dec 18
Close button (see top left of custom tab bar)
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c684323b978e024b67bcd645b61b1e8a35b7138b commit c684323b978e024b67bcd645b61b1e8a35b7138b Author: Jay Harris <harrisjay@chromium.org> Date: Wed Dec 19 05:55:22 2018 Adds a 'Close' button to the custom tab bar view. The close button navigates back to the last in scope url, if any. If no in scope url is available (as when using window.location.replace(...)) the app's launch_url is navigated to and the history stack is cleared. Notes: - In future, the forward stack should be cleared when the close button is clicked. Screenshot: https://bugs.chromium.org/p/chromium/issues/attachment?aid=372336&signed_aid=Zqm633OoKgTghf6XijsROQ==&inline=1 Bug: 853593 Change-Id: I0868500e6a98445ed5113b61bb6a7529bccb7d2c Reviewed-on: https://chromium-review.googlesource.com/c/1381391 Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Jay Harris <harrisjay@chromium.org> Cr-Commit-Position: refs/heads/master@{#617736} [modify] https://crrev.com/c684323b978e024b67bcd645b61b1e8a35b7138b/chrome/browser/ui/views/location_bar/custom_tab_bar_view.cc [modify] https://crrev.com/c684323b978e024b67bcd645b61b1e8a35b7138b/chrome/browser/ui/views/location_bar/custom_tab_bar_view.h [modify] https://crrev.com/c684323b978e024b67bcd645b61b1e8a35b7138b/ui/views/controls/button/image_button_factory.cc [modify] https://crrev.com/c684323b978e024b67bcd645b61b1e8a35b7138b/ui/views/controls/button/image_button_factory.h
,
Dec 20
Adds larger screenshot, to make change more obvious. Note: this does not effect the window border itself, it stops lines from being drawn between the border and the toolbar. This will align the Linux custom tab bars with ones on other platforms.
,
Dec 20
Pictures of other platforms, for comparison
,
Dec 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9d37d5147d8b9d9a20bd1b55f2f7daad06ed0c3e commit 9d37d5147d8b9d9a20bd1b55f2f7daad06ed0c3e Author: Jay Harris <harrisjay@chromium.org> Date: Fri Dec 21 00:42:40 2018 Stop drawing border between window frame and toolbar in PWAs. The custom tab bar should appear as an extension of the title bar. Before this CL, this was not not the case on Linux or Windows without DWM (disable-dwm-composition), as we draw small borders between the window frame and the toolbar. This CL removes them when displaying the custom tab bar. Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=372746&signed_aid=ybrF8gUqHfRhz-mI-R7-SQ==&inline=1 After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=372747&signed_aid=-_9BQ_5YxzT6Gg67Aj49ag==&inline=1 Bug: 853593 Change-Id: Ice90aef02c074d344066f8b5dd8e50fe77310a8e Reviewed-on: https://chromium-review.googlesource.com/c/1385685 Commit-Queue: Jay Harris <harrisjay@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#618404} [modify] https://crrev.com/9d37d5147d8b9d9a20bd1b55f2f7daad06ed0c3e/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
,
Jan 21
(2 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f3a6c253c7ae2eb42dd58de281587dbf110ef493 commit f3a6c253c7ae2eb42dd58de281587dbf110ef493 Author: Jay Harris <harrisjay@chromium.org> Date: Mon Jan 21 04:01:20 2019 Updates the bottom border to be 15% black instead of black. This is a UX issue that came up during https://crbug.com/918788 Bug: 853593 Change-Id: If0efdba2d344ee379fb560c61dc43ea4a14c0510 Reviewed-on: https://chromium-review.googlesource.com/c/1415232 Commit-Queue: Jay Harris <harrisjay@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/master@{#624515} [modify] https://crrev.com/f3a6c253c7ae2eb42dd58de281587dbf110ef493/chrome/browser/ui/views/location_bar/custom_tab_bar_view.cc
,
Yesterday
(40 hours ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4205d6e8d3370bc816d722e908a372921936dd8a commit 4205d6e8d3370bc816d722e908a372921936dd8a Author: Gabriel Charette <gab@chromium.org> Date: Mon Jan 21 18:46:27 2019 Revert "Updates the bottom border to be 15% black instead of black." This reverts commit f3a6c253c7ae2eb42dd58de281587dbf110ef493. Reason for revert: suspected for https://bugs.chromium.org/p/chromium/issues/detail?id=923894#c14 Original change's description: > Updates the bottom border to be 15% black instead of black. > > This is a UX issue that came up during https://crbug.com/918788 > > Bug: 853593 > Change-Id: If0efdba2d344ee379fb560c61dc43ea4a14c0510 > Reviewed-on: https://chromium-review.googlesource.com/c/1415232 > Commit-Queue: Jay Harris <harrisjay@chromium.org> > Reviewed-by: Bret Sepulveda <bsep@chromium.org> > Cr-Commit-Position: refs/heads/master@{#624515} TBR=bsep@chromium.org,harrisjay@chromium.org Change-Id: I7ff365b2fcfafdbd75f6bd506c56f07ab806579d No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 853593, 923894 Reviewed-on: https://chromium-review.googlesource.com/c/1425997 Reviewed-by: Gabriel Charette <gab@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#624635} [modify] https://crrev.com/4205d6e8d3370bc816d722e908a372921936dd8a/chrome/browser/ui/views/location_bar/custom_tab_bar_view.cc
,
Yesterday
(38 hours ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c44ee8b46204c637a34012809857363ea88aac2f commit c44ee8b46204c637a34012809857363ea88aac2f Author: Gabriel Charette <gab@chromium.org> Date: Mon Jan 21 21:25:59 2019 Reland "Updates the bottom border to be 15% black instead of black." This reverts commit 4205d6e8d3370bc816d722e908a372921936dd8a. Reason for reland : didn't fix it. Original change's description: > Revert "Updates the bottom border to be 15% black instead of black." > > This reverts commit f3a6c253c7ae2eb42dd58de281587dbf110ef493. > > Reason for revert: suspected for https://bugs.chromium.org/p/chromium/issues/detail?id=923894#c14 > > Original change's description: > > Updates the bottom border to be 15% black instead of black. > > > > This is a UX issue that came up during https://crbug.com/918788 > > > > Bug: 853593 > > Change-Id: If0efdba2d344ee379fb560c61dc43ea4a14c0510 > > Reviewed-on: https://chromium-review.googlesource.com/c/1415232 > > Commit-Queue: Jay Harris <harrisjay@chromium.org> > > Reviewed-by: Bret Sepulveda <bsep@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#624515} > > TBR=bsep@chromium.org,harrisjay@chromium.org > > Change-Id: I7ff365b2fcfafdbd75f6bd506c56f07ab806579d > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: 853593, 923894 > Reviewed-on: https://chromium-review.googlesource.com/c/1425997 > Reviewed-by: Gabriel Charette <gab@chromium.org> > Commit-Queue: Gabriel Charette <gab@chromium.org> > Cr-Commit-Position: refs/heads/master@{#624635} TBR=gab@chromium.org,bsep@chromium.org,harrisjay@chromium.org Change-Id: I6011811309c67dde14d7e390f8aabba54c5db111 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 853593, 923894 Reviewed-on: https://chromium-review.googlesource.com/c/1426013 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#624677} [modify] https://crrev.com/c44ee8b46204c637a34012809857363ea88aac2f/chrome/browser/ui/views/location_bar/custom_tab_bar_view.cc |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by ortuno@chromium.org
, Jun 18 2018