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

Issue 720120 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug

Blocking:
issue 690611



Sign in to add a comment

Touch bar during full screen does not show URL scheme.

Project Member Reported by lgar...@chromium.org, May 9 2017

Issue description

Chrome 60.0.3088.3
OSX 10.12.4

What steps will reproduce the problem?
(1) Visit https://permission.site/ on a MacBook Pro with a touch bar.
(2) Press "Fullscreen"

What is the expected result?
The touch bar URL contains the scheme, at least for HTTPS URLs (like the omnibox).

What happens instead?
The scheme is hidden.

spqchan@, could you triage?
 
Touch Bar Shot 2017-05-09 at 14.43.49.png
17.6 KB View Download
Cc: shrike@chromium.org bettes@chromium.org
The scheme is hidden by design. See the attached image.

Adding shrike@ and bettes@ for their opinion
ExitFullScreen.png
190 KB View Download
What is the rationale?

If we're showing the domain, presumably we should hide the path.
If we're showing the URL, the scheme/security state is an important part. We require it of all Chrome-controlled surfaces that show a URL.

There are exceptions, for surfaces can only show HTTPS URLs, but fullscreen is available over HTTP, so this is unfortunately not one of those surfaces.

Comment 3 by shrike@chromium.org, May 10 2017

Cc: spqc...@chromium.org
Labels: -Pri-3 M-60 OS-Mac Pri-2
Owner: bettes@chromium.org
Security-wise it seems like the design should incorporate the scheme. Assigning to bettes@ for a rework. If UX has strong preference for not showing the scheme we can all meet to discuss.
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, May 24 2017

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

commit c5cad2b1b7a9517033daf9e14880a7643818da57
Author: spqchan <spqchan@chromium.org>
Date: Wed May 24 01:56:57 2017

[Mac] Add URL Scheme to Fullscreen Touch Bar

Bug:  720120 
Change-Id: I6652b218306e7b864a73d3936433b84e9b035a8a
Reviewed-on: https://chromium-review.googlesource.com/513384
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#474128}
[modify] https://crrev.com/c5cad2b1b7a9517033daf9e14880a7643818da57/chrome/browser/ui/cocoa/browser_window_touch_bar.mm

Comment 6 by shrike@chromium.org, May 26 2017

Labels: -Pri-2 ReleaseBlock-Stable Pri-1
Owner: spqc...@chromium.org
spqchan@ - I tried this on my Touch Bar Mac and saw that the last five characters of the URL are now drawn in a different gray than the rest of the URL.
I'm already working on a fix for it
Project Member

Comment 8 by bugdroid1@chromium.org, May 27 2017

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

commit 21936988effb209f042e8484dfc27f5b4189016c
Author: spqchan <spqchan@chromium.org>
Date: Sat May 27 01:28:08 2017

[Mac] Fix the URL on the Fullscreen Touch Bar

Calculate the url's path index with GetEmptyPath()
since the previous method wouldn't play nicely with
queries and etc.

Bug:  720120 
Change-Id: Icbd609e02d43f043f75b2614ee5ffcc68497a4a8
Reviewed-on: https://chromium-review.googlesource.com/517503
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#475203}
[modify] https://crrev.com/21936988effb209f042e8484dfc27f5b4189016c/chrome/browser/ui/cocoa/browser_window_touch_bar.mm

Comment 9 by vku...@etouch.net, May 30 2017

NOTE:
Checked above issue on Macbook pro Touchbar in latest dev 60.0.3112.7 build and observed that the url scheme is not seen.
Labels: Needs-Feedback
@spqchan -- As per Comment# 9, please let us know whether this is the desired behavior so that verified labels could be added accordingly.
Thanks in Advance.
I just checked, the scheme is there. Are you entering fullscreen via youtube or some other tab content?

Anyway, I fixed that issue, but just realized that the slash also needs to be greyed out. I'm working on it
Cc: ligim...@chromium.org
Labels: -Needs-Feedback
The above change landed in 61.0.3114.0. vkupte@, please check in latest canary.

Sarah, if all looks good please request a merge to M60 when you are ready.
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 6 2017

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

commit 7aedf600f25ab27208dd388d287161d207807ac4
Author: spqchan <spqchan@chromium.org>
Date: Tue Jun 06 06:29:19 2017

[Mac] Fullscreen Touch Bar URL Adjustments

- Remove trailing slash
- Highlight the slash along with the rest of the path

Bug:  720120 
Change-Id: I5e342f05d74966138cfa94c37fc694b411d2b0e9
Reviewed-on: https://chromium-review.googlesource.com/521903
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477215}
[modify] https://crrev.com/7aedf600f25ab27208dd388d287161d207807ac4/chrome/browser/ui/cocoa/browser_window_touch_bar.mm

With respect to comment #12:
Above issue is still reproducible(i.e. Url scheme is not seen on Touch toolbar) on Latest Canary Version:61.0.3123.0 using Macbook pro Touchbar
abombde@etouch.net - are you following the steps in the problem description?
With response to comment #15:

Followed step provided/mentioned in problem description, But still above issue is not fixed (i.e.Url scheme is not seen beside 'Esc' button on Touch toolbar) on Latest Chrome Version:61.0.3124.4 (Official Build) 2ccba3fd9c04d6ca967808d2fbb9aadbbda5fb6f-refs/branch-heads/3124@{#6} using Macbook pro Touchbar(10.12.1)

Thanks.
Labels: Needs-Feedback
abombde@etouch.net - please provide an exact numbered set of steps to reproduce the problem, as well as a photo of the Touch Bar. I'm not sure what you're seeing on the Touch Bar, and I want to confirm (again) how you're getting there.
Labels: Merge-Request-60
Just tested this in Canary, it works fine.

Note: please make sure you're pressing the "Fullscreen" button on the web page, not on the browser.

Alternatively, you can pick a youtube video and press the video's fullscreen button
Project Member

Comment 20 by sheriffbot@chromium.org, Jun 9 2017

Labels: -Merge-Request-60 Hotlist-Merge-Approved Merge-Approved-60
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 21 by bugdroid1@chromium.org, Jun 9 2017

Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a64899eae8fa75447b24051b3f5a1add3a2000ef

commit a64899eae8fa75447b24051b3f5a1add3a2000ef
Author: spqchan <spqchan@chromium.org>
Date: Fri Jun 09 18:38:37 2017

[Mac] Fix the URL on the Fullscreen Touch Bar

Calculate the url's path index with GetEmptyPath()
since the previous method wouldn't play nicely with
queries and etc.

Bug:  720120 
Change-Id: Icbd609e02d43f043f75b2614ee5ffcc68497a4a8
Reviewed-on: https://chromium-review.googlesource.com/517503
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#475203}
Review-Url: https://codereview.chromium.org/2928153003 .
Cr-Commit-Position: refs/branch-heads/3112@{#291}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

[modify] https://crrev.com/a64899eae8fa75447b24051b3f5a1add3a2000ef/chrome/browser/ui/cocoa/browser_window_touch_bar.mm

Project Member

Comment 22 by bugdroid1@chromium.org, Jun 9 2017

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

commit e8f0c348e871eed0c1b85128c7fde35029dce8f6
Author: spqchan <spqchan@chromium.org>
Date: Fri Jun 09 20:07:12 2017

[Mac] Fullscreen Touch Bar URL Adjustments

- Remove trailing slash
- Highlight the slash along with the rest of the path

Bug:  720120 
Change-Id: I5e342f05d74966138cfa94c37fc694b411d2b0e9
Reviewed-on: https://chromium-review.googlesource.com/521903
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#477215}
Review-Url: https://codereview.chromium.org/2933583002 .
Cr-Commit-Position: refs/branch-heads/3112@{#298}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

[modify] https://crrev.com/e8f0c348e871eed0c1b85128c7fde35029dce8f6/chrome/browser/ui/cocoa/browser_window_touch_bar.mm

Status: Fixed (was: Started)

Sign in to add a comment