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

Issue 626992 link

Starred by 3 users
Status: Fixed
Owner:
wangxianzhu@chromium.org
Closed: Jul 2016
Cc:
nyerramilli@chromium.org
█ msrchandra@chromium.org
█ rnimmagadda@chromium.org
vmp...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Regression : Traces of blue focus highlight is seen in www.account.samsung.com

Reported by mni...@etouch.net, Jul 11 2016 
Version: 54.0.2793.0 6667812b2dffe3f6bf794b2b040504e5a3ad7cb7-refs/heads/master@{#404564} (32/64-bit)
OS:  Windows (7,8,8.1,10),Linux (14.04 LTS)
URL : https://account.samsung.com/account/find/accountFindEmailView.do?serviceID=tj9u972o46

What steps will reproduce the problem?
1) Launch chrome and navigate to above url
2) Now press 'Tab' key from keyboard and observe the blue focus highlight below 'Samsung account'

Actual : Traces of blue focus highlight is seen below 'Samsung account' text
Expected : Traces of blue focus highlight should not be seen below 'Samsung account' text

This is a regression issue broken in 'M-52' and below is the manual regression and Narrow bisect info:
Good build : 52.0.2730.0
Bad build : 52.0.2731.0

Narrow bisect info:
https://chromium.googlesource.com/chromium/src/+log/a69b588abf1afd1e37e58463a07df4da722ded02..6958c09a512167e272ba30c3d1caeea283a803da?pretty=fuller&n=100

Suspecting : r392716 from Narrow bisect 

@iclelland : Could you please help to reassign if your change is not the cause for this change.

Note : Issue is not seen on Mac OS.
Actual_video.mp4
695 KB View Download
Expected_video.mp4
688 KB View Download
Actual_screenshot.png
274 KB View Download
Expected_screenshot.png
274 KB View Download

Comment 1 by iclell...@chromium.org, Jul 11 2016

Owner: wangxianzhu@chromium.org
This definitely isn't my change -- that change only disabled a *layout test*.

I ran a bisect across the commits in that range to narrow it down more, and it appears to start for me with crrev.com/638c1a24. ("Don't include contents visual overflow into paint invalidation rect")

I can reproduce it pretty reliably from commits 638c1a24 and onward. Just loading that page, and using tab and shift-tab in sequence a few time is enough to get the bottom of the outline around the samsung logo either persistent or completely missing.

Reassigning to wangxianzhu for investigation.

Comment 2 by wangxianzhu@chromium.org, Jul 11 2016

Components: -Blink>Compositing Blink>Paint>Invalidation
Labels: -Pri-1 Pri-2
Reduced test case. The bug reproduces only when the page is in "almost standards" or quirks mode.

Lowering priority because this bug is in non-standard mode.
outline-invalidation.html
314 bytes View Download
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 13 2016 

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

commit b62027f56fdb4906393fe60105d33194e907624a
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Wed Jul 13 01:10:03 2016

Fix LayoutInline::visualOverflowRect() with outline in non-standard mode

In non-standard mode, LayoutInline::linesVisualOverflowBoundingBox()
doesn't necessarily cover outline rects of line boxes because we treat
the line height differently when a line contains replaced elements
in LayoutBlock::minLineHeightForReplacedObject().

BUG= 626992 
TEST=paint/invalidation/outline-containing-image-in-non-standard-mode.html

Review-Url: https://codereview.chromium.org/2134413002
Cr-Commit-Position: refs/heads/master@{#404869}

[modify] https://crrev.com/b62027f56fdb4906393fe60105d33194e907624a/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/b62027f56fdb4906393fe60105d33194e907624a/third_party/WebKit/LayoutTests/paint/invalidation/outline-containing-image-in-non-standard-mode-expected.html
[add] https://crrev.com/b62027f56fdb4906393fe60105d33194e907624a/third_party/WebKit/LayoutTests/paint/invalidation/outline-containing-image-in-non-standard-mode-expected.txt
[add] https://crrev.com/b62027f56fdb4906393fe60105d33194e907624a/third_party/WebKit/LayoutTests/paint/invalidation/outline-containing-image-in-non-standard-mode.html
[modify] https://crrev.com/b62027f56fdb4906393fe60105d33194e907624a/third_party/WebKit/Source/core/layout/LayoutInline.cpp

Comment 4 by wangxianzhu@chromium.org, Jul 13 2016

Labels: -M-52 Merge-Request-53 M-53

Comment 7 by dimu@google.com, Jul 14 2016

Labels: -Merge-Request-53 Merge-Review-53 Hotlist-Merge-Review
[Automated comment] There appears to be on-going work (i.e. bugroid changes), needs manual review.

Comment 8 by gov...@chromium.org, Jul 14 2016 

Before we approve merge to M53, Could you please confirm whether this change is baked/verified in Canary and safe to merge?

Comment 9 by wangxianzhu@chromium.org, Jul 14 2016 

Confirmed.

Comment 10 by gov...@chromium.org, Jul 14 2016

Labels: -Merge-Review-53 Merge-Approved-53
Thank you. Approving merge to M53 branch 2785 based on comment #9. Please merge asap. T
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 14 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7d6c4cd3cb59d246011cc3301e89fb9bafe37609

commit 7d6c4cd3cb59d246011cc3301e89fb9bafe37609
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Thu Jul 14 16:50:50 2016

Fix LayoutInline::visualOverflowRect() with outline in non-standard mode

In non-standard mode, LayoutInline::linesVisualOverflowBoundingBox()
doesn't necessarily cover outline rects of line boxes because we treat
the line height differently when a line contains replaced elements
in LayoutBlock::minLineHeightForReplacedObject().

BUG= 626992 
TEST=paint/invalidation/outline-containing-image-in-non-standard-mode.html

Review URL: https://codereview.chromium.org/2152823002 .

Review-Url: https://codereview.chromium.org/2134413002
Cr-Original-Commit-Position: refs/heads/master@{#404869}
Cr-Commit-Position: refs/branch-heads/2785@{#113}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/7d6c4cd3cb59d246011cc3301e89fb9bafe37609/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/7d6c4cd3cb59d246011cc3301e89fb9bafe37609/third_party/WebKit/LayoutTests/paint/invalidation/outline-containing-image-in-non-standard-mode-expected.html
[add] https://crrev.com/7d6c4cd3cb59d246011cc3301e89fb9bafe37609/third_party/WebKit/LayoutTests/paint/invalidation/outline-containing-image-in-non-standard-mode-expected.txt
[add] https://crrev.com/7d6c4cd3cb59d246011cc3301e89fb9bafe37609/third_party/WebKit/LayoutTests/paint/invalidation/outline-containing-image-in-non-standard-mode.html
[modify] https://crrev.com/7d6c4cd3cb59d246011cc3301e89fb9bafe37609/third_party/WebKit/Source/core/layout/LayoutInline.cpp

Comment 12 by wangxianzhu@chromium.org, Jul 14 2016

Status: Fixed (was: Assigned)

Comment 13 by rnimmagadda@chromium.org, Jul 19 2016

Cc: rnimmagadda@chromium.org
Labels: TE-Verified-M53 TE-Verified-53.0.2785.21
Verified the fix on Windows 7 & Ubuntu Trusty (14.04) for Google Chrome Dev Version - 53.0.2785.21 

Screen-shots are attached.

TE-Verified labels are added.
626992.mp4
1.3 MB View Download
►

Sign in to add a comment