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

Issue 731598 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

background color removed from first cells when using filter:brightness with parent that has margin-left and overflow-x:auto

Reported by vnexs...@gmail.com, Jun 9 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.86 Safari/537.36

Steps to reproduce the problem:
http://jsbin.com/jimonug/edit?html,output

explanation from the link above:

in order to get this problem you need a table with a row that has background color and filter:brightness and also a parent div with margin-left and overflow-x:auto.

on the 2nd row, the background color is removed on the first 3 cells, we can modify the amount of cells with background cleared by modifying the margin-left on the #demoPage; in order to get this problem we need the margin-left, the table's parent div with overflow-x:auto, and the filter: brightness(x) on the row.

What is the expected behavior?
background color of the cells should not be cleared

What went wrong?
background color of the first cells was cleared

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 59.0.3071.86  Channel: stable
OS Version: 10.0
Flash Version:
 
filterbrightmargin.html
2.6 KB View Download
Cc: chrishtr@chromium.org abdulsyed@chromium.org wkorman@chromium.org
Components: -Blink>HTML Blink>Paint
Labels: -Type-Bug -Pri-2 ReleaseBlock-Stable pre-stable-59.0.3071.86 M-59 OS-Linux OS-Mac Pri-1 Type-Bug-Regression
Owner: wangxianzhu@chromium.org
Status: Assigned (was: Unconfirmed)
This is the regression which started in M59, please find bisect results below :

You are probably looking for a change made after 461906 (known good), but no lat
er than 461907 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/b226550dbd1834c2f7addec9d1df7d57498fb7fc..62ad1c5bc2e538f2a57c2e16e06327414a6fbf7a


Note : I am marking the bug as stable blocker for now.
Labels: OS-Android
tagging Android since this is Blink regression.
Labels: -M-59 M-60
Remove M-59 stable release blocker because impact is too low to block the current roll-out and the fix is likely to be more risky than we would merge.

But leaving it as an M-60 stable blocker because extensions use brightness filters for various purposes and we don't want to leave such cases broken.
Just to update the latest behaviour,
Able to reproduce the issue on Mac 10.12.5 using latest canary #61.0.3130.0.

wangxianzhu@ - Gentle Ping...!!
Could you please have a look into the issue as it has been marked as a stable blocker.

Thanks...!!
I will fix this before M60 stable release.
Project Member

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

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

commit bdb7aad96a13cf54358ad15ebb7dec787e478f77
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Thu Jun 15 18:28:38 2017

Fix background painting of row as layer with paint offset

The dirtied-columns calculation was incorrect in the case (due to
incorrect coordinates of the cull rect), causing the row background
is not painted behind some cells.

Remove the dirtied-columns logic. Now it's the same as what we did
before https://codereview.chromium.org/2786463004.

Justification of removing the logic about performance:
- Self-painting rows are rare;
- In a large table containing many self-painting rows, we won't paint
  the rows out of the interest rect.
- We didn't have performance issues reported when we didn't have the
  logic;

BUG= 731598 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I9a452feefb6d2f4121729f28bb52835775f025f8
Reviewed-on: https://chromium-review.googlesource.com/535084
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479763}
[add] https://crrev.com/bdb7aad96a13cf54358ad15ebb7dec787e478f77/third_party/WebKit/LayoutTests/paint/tables/stacking-context-row-background-clipped-with-offset-expected.html
[add] https://crrev.com/bdb7aad96a13cf54358ad15ebb7dec787e478f77/third_party/WebKit/LayoutTests/paint/tables/stacking-context-row-background-clipped-with-offset.html
[modify] https://crrev.com/bdb7aad96a13cf54358ad15ebb7dec787e478f77/third_party/WebKit/Source/core/paint/TableRowPainter.cpp

Labels: Merge-Request-60
Project Member

Comment 8 by sheriffbot@chromium.org, Jun 16 2017

Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: M60 has already been promoted to the beta branch, so this requires manual review
Please contact the 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
Labels: -Merge-Review-60 Merge-Approved-60
Fix to a RB-Stable bug. Approving merge to M60. 
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 19 2017

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

commit f90cd5f100347b6140cbf94661aed15132065c4c
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Mon Jun 19 03:15:09 2017

Fix background painting of row as layer with paint offset

The dirtied-columns calculation was incorrect in the case (due to
incorrect coordinates of the cull rect), causing the row background
is not painted behind some cells.

Remove the dirtied-columns logic. Now it's the same as what we did
before https://codereview.chromium.org/2786463004.

Justification of removing the logic about performance:
- Self-painting rows are rare;
- In a large table containing many self-painting rows, we won't paint
  the rows out of the interest rect.
- We didn't have performance issues reported when we didn't have the
  logic;

BUG= 731598 
TBR=wangxianzhu@chromium.org

(cherry picked from commit bdb7aad96a13cf54358ad15ebb7dec787e478f77)

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I9a452feefb6d2f4121729f28bb52835775f025f8
Reviewed-on: https://chromium-review.googlesource.com/535084
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#479763}
Reviewed-on: https://chromium-review.googlesource.com/538988
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/branch-heads/3112@{#379}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}
[add] https://crrev.com/f90cd5f100347b6140cbf94661aed15132065c4c/third_party/WebKit/LayoutTests/paint/tables/stacking-context-row-background-clipped-with-offset-expected.html
[add] https://crrev.com/f90cd5f100347b6140cbf94661aed15132065c4c/third_party/WebKit/LayoutTests/paint/tables/stacking-context-row-background-clipped-with-offset.html
[modify] https://crrev.com/f90cd5f100347b6140cbf94661aed15132065c4c/third_party/WebKit/Source/core/paint/TableRowPainter.cpp

Labels: TE-Verified-M61 TE-Verified-61.0.3138.0
Verified the issue on windows 10, Mac 10.12.5 and Ubuntu 14.04 using latest chrome  version #61.0.3138.0 as per comment #0.

Observed that background color of the cells did not get cleared. Hence, the fix is working as expected.
Attaching screen shot for reference.
Hence, adding the verified labels.

Thanks...!!
paint.JPG
137 KB View Download
Status: Verified (was: Assigned)

Sign in to add a comment