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

Issue 866668 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Taller tabs mean some themes have gaps below tabs

Project Member Reported by pkasting@chromium.org, Jul 23

Issue description

Some themes have gaps below the background tabs because the taller tab height means the themes' tab background images don't reach to the bottom.

Fix: In such cases, vertically tile the tab background image, mirrored.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 26

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

commit b5a1628b71a1b431130833dd193dc1f74a4358aa
Author: Peter Kasting <pkasting@chromium.org>
Date: Thu Jul 26 04:04:30 2018

Mirror the inactive tab background image vertically if necessary.

Refresh makes tabs taller, and some themes don't provide a background image of
sufficient height.  Tiling by repeating risks looking bad for themes that e.g.
put a line across the top of tabs.  Mirroring can look bad in a lot of cases
too, but it's better than tiling or doing nothing.

Most of this patch is plumbing to expose per-axis tiling modes.

Bug:  866668 
Change-Id: Ia6e00a15146e3eb32cda3c8362de4e83e67a8b7f
Reviewed-on: https://chromium-review.googlesource.com/1147740
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578190}
[modify] https://crrev.com/b5a1628b71a1b431130833dd193dc1f74a4358aa/ash/wallpaper/wallpaper_view.cc
[modify] https://crrev.com/b5a1628b71a1b431130833dd193dc1f74a4358aa/chrome/browser/themes/browser_theme_pack.cc
[modify] https://crrev.com/b5a1628b71a1b431130833dd193dc1f74a4358aa/chrome/browser/ui/views/tabs/new_tab_button.cc
[modify] https://crrev.com/b5a1628b71a1b431130833dd193dc1f74a4358aa/ui/gfx/canvas.cc
[modify] https://crrev.com/b5a1628b71a1b431130833dd193dc1f74a4358aa/ui/gfx/canvas.h
[modify] https://crrev.com/b5a1628b71a1b431130833dd193dc1f74a4358aa/ui/gfx/skia_paint_util.cc
[modify] https://crrev.com/b5a1628b71a1b431130833dd193dc1f74a4358aa/ui/gfx/skia_paint_util.h

Labels: Merge-Request-69
Project Member

Comment 3 by sheriffbot@chromium.org, Jul 29

Labels: -Merge-Request-69 Hotlist-Merge-Approved Merge-Approved-69
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

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

Please merge your change to M69 branch 3497 by 2:00 PM PT Monday, 07/30, so we can pick it up for next week last M69 Dev release. Thank you.

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 30

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a05dbee5c5200973e71a65c3b18c6703606cafa7

commit a05dbee5c5200973e71a65c3b18c6703606cafa7
Author: Peter Kasting <pkasting@chromium.org>
Date: Mon Jul 30 20:36:34 2018

Mirror the inactive tab background image vertically if necessary.

Refresh makes tabs taller, and some themes don't provide a background image of
sufficient height.  Tiling by repeating risks looking bad for themes that e.g.
put a line across the top of tabs.  Mirroring can look bad in a lot of cases
too, but it's better than tiling or doing nothing.

Most of this patch is plumbing to expose per-axis tiling modes.

Bug:  866668 
Change-Id: Ia6e00a15146e3eb32cda3c8362de4e83e67a8b7f
Reviewed-on: https://chromium-review.googlesource.com/1147740
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#578190}(cherry picked from commit b5a1628b71a1b431130833dd193dc1f74a4358aa)
Reviewed-on: https://chromium-review.googlesource.com/1155607
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#238}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/a05dbee5c5200973e71a65c3b18c6703606cafa7/ash/wallpaper/wallpaper_view.cc
[modify] https://crrev.com/a05dbee5c5200973e71a65c3b18c6703606cafa7/chrome/browser/themes/browser_theme_pack.cc
[modify] https://crrev.com/a05dbee5c5200973e71a65c3b18c6703606cafa7/chrome/browser/ui/views/tabs/new_tab_button.cc
[modify] https://crrev.com/a05dbee5c5200973e71a65c3b18c6703606cafa7/ui/gfx/canvas.cc
[modify] https://crrev.com/a05dbee5c5200973e71a65c3b18c6703606cafa7/ui/gfx/canvas.h
[modify] https://crrev.com/a05dbee5c5200973e71a65c3b18c6703606cafa7/ui/gfx/skia_paint_util.cc
[modify] https://crrev.com/a05dbee5c5200973e71a65c3b18c6703606cafa7/ui/gfx/skia_paint_util.h

Status: Fixed (was: Started)
Labels: Needs-Feedback
pkasting@ : Could you please provide manual repro steps for verification of fix.

Thanks.!
Install the Typography theme and see if the blue background for background tabs goes all the way to the toolbar.
Cc: abdulsyed@chromium.org
+abdulsyed@ fyi, M69 merges taken for Proj-MdRefresh .

Sign in to add a comment