New issue
Advanced search Search tips

Issue 863493 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

[MDRefresh] Regression: Tab Favicon/Title/Close Button moved 1px down

Project Member Reported by meh...@chromium.org, Jul 13

Issue description

Chrome Version: 69.0.3489.0 Canary
OS: macOS but probably OS=All

What steps will reproduce the problem?
(1) Open two New Tabs
(2) Take a look at the Favicon/Title/Close Button and the NewTabButton


What is the expected result?
Tab Favicon/Title/Close Button should be 1px higher so that the middle of the Close button is be aligned with the horizontal line New Tab Button.


What happens instead?
Tab Favicon/Title/Close Button moved 1px down.


This is a regression.

I did a bisect: https://chromium.googlesource.com/chromium/src/+log/a18e8a4a19f2f5dd2d52b8d715da8fa61a3b9654..d82ee0010741390a1a6a63bbcc68d95e4b5341b2

Probably caused by https://chromium-review.googlesource.com/c/chromium/src/+/1121386

tbergquist@: Can you please take a look at this issue, if it is caused by your change?

Thanks :)
Mehmet

 
actual.png
9.8 KB View Download
expected.png
12.4 KB View Download
Blargh.  Yeah, this is Taylor's change.  We need to include the overlap height in Tab::GetContentsInsets(); that should fix it.
Yeah, woops, this does seem like a predictable outcome of my change... https://crrev.com/c/1137353 does what Peter suggested, and it does indeed fix it.
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 16

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

commit 4747cc23ae334a57a35ed3c8e6adcdbc8a50d479
Author: Taylor Bergquist <tbergquist@chromium.org>
Date: Mon Jul 16 22:50:44 2018

Fix tab content being one pixel too low

In https://crrev.com/c/1121386 the tab's layout height was increased by
one dip to create an overlap between the tab and the toolbar.  The tab
content was then incorrectly laid out slightly lower as a result.  This
CL includes that space in the tab's content insets, so content is laid
out into the visible tab region as intended.

Bug:  863493 
Change-Id: Iae13fb5bc9a726a1b9f29329ebecd23197b2b0fb
Reviewed-on: https://chromium-review.googlesource.com/1137353
Commit-Queue: Taylor Bergquist <tbergquist@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575458}
[modify] https://crrev.com/4747cc23ae334a57a35ed3c8e6adcdbc8a50d479/chrome/browser/ui/views/tabs/tab.cc

Labels: TE-Verified-M69 TE-Verified-69.0.3494.0
Able to reproduce the issue on chrome version 69.0.3489.0 (build without fix) as per the comment #0.
Verified the fix on Mac 10.13.5, Windows 10 and Ubuntu 17.10  using Chrome version # 69.0.3494.0 .
Attaching screen-shot for reference.
Observed that  "The middle of the Close button is be aligned with the horizontal line New Tab Button. " 
The fix is working as expected, adding Verified labels

Thanks...!
863493.png
133 KB View Download
863493-1.png
119 KB View Download
Labels: -Pri-1 Pri-3 Hotlist-Polish
Owner: kylixrd@chromium.org
Reassigning to kylixrd for now due to height changes.
Status: Fixed (was: Assigned)
I'm confused -- this is Fixed.

Sign in to add a comment