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

Issue 836127 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-05-03
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression

Blocking:
issue 771643



Sign in to add a comment

SVG clip path is ignored after fill change

Reported by jan.boes...@incors.com, Apr 24 2018

Issue description

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

Example URL:

Steps to reproduce the problem:
1.Open the attached file or the codepen at https://codepen.io/anon/pen/QrNwoa?editors=1010
2. Click on the "Change Color" botton

Clicking on the button will change the fill of the rectangle to 'SteelBlue'

What is the expected behavior?
Only the color of the rounded rectangle should change.

What went wrong?
The clip is ignored and the whole original element is visible and filled with the new color.
This issue only occurs if
A mask and a filter is applied to the original element
AND
The clip is not set directly on the original element but on the containing group element.

Does it occur on multiple sites: N/A

Is it a problem with a plugin? No 

Did this work before? Yes 65.0.3325.181

Does this work in other browsers? Yes

Chrome version: 68.0.3405.0  Channel: n/a
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version:
 
svg-clipping-issue.html
1.8 KB View Download

Comment 1 by woxxom@gmail.com, Apr 24 2018

Bisect info: 543288 (good) - 543294 (bad)
https://chromium.googlesource.com/chromium/src/+log/72b68bc8..209644b1?pretty=fuller
Suspecting r543291 = 3a3c78a924a686ed0d3f90d765b00cdd78453e11 = https://crrev.com/c/947902 by wangxianzhu@chromium.org
"Reland "[SPv175] Enable SlimmingPaintV175 by default"
Landed in 67.0.3372.0

Confirmed by disabling the feature via command line and observing the bug is gone:
chrome --disable-blink-features=SlimmingPaintV175

Bisected the underlying bug using the following command line: "--enable-blink-features=SlimmingPaintV175"
1dd12d1e64b4f5777d5a1623700572f8dd9a3d4b "[Blink] Add new mode to use property tree to paint in SPv1"
Landed in 63.0.3215.0 (disabled by default)
Labels: Needs-Triage-M68 Needs-Bisect
Cc: susan.boorgula@chromium.org
Components: Blink>Paint
Labels: -Pri-2 -Type-Compat -Needs-Bisect Target-67 M-68 Triaged-ET RegressedIn-67 ReleaseBlock-Stable FoundIn-67 Target-68 FoundIn-68 hasbisect OS-Linux OS-Mac Pri-1 Type-Bug-Regression
Owner: wangxianzhu@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce this issue on Windows 10, Mac OS 10.13.1 and Ubuntu 17.10 on the latest Canary 68.0.3405.0 and latest dev 67.0.3396.10 as per the original comment.

Bisect Information:
===================
Good Build: 67.0.3371.0 (Revision - 543278)
Bad Build : 67.0.3372.0 (Revision - 543592)

As per comment #1, the CL which caused this issue is 
Reviewed-on: https://chromium-review.googlesource.com/923572

wangxianzhu@ Please check and confirm if this issue is related to your change, else help us in assigning to the right owner.
Adding ReleaseBlock-Stable as this is a recent regression. Please feel free to remove the same if it is not applicable.

Thanks
Cc: wangxianzhu@chromium.org
Owner: trchen@chromium.org
trchen@ can you take a look at this clip-path issue?
This seems to be fixed in 68.0.3409.0

Comment 6 by woxxom@gmail.com, Apr 26 2018

>This seems to be fixed in 68.0.3409.0

Still broken here.
I guess your browser simply received a different field trial variation for SlimmingPaintV175.
>Still broken here.

You are correct, after restarting Canary the bug is back.
Blocking: 771643
Labels: -M-68 M-67

Comment 9 by trchen@chromium.org, Apr 28 2018

Status: Started (was: Assigned)
Just tried to repro this locally today. This bug only repros in non-DCHECK builds. This implies some of our DCHECK actually introduced side effects, which is very very bad. I will be investigating this as my top priority.
The code in find_properties_needing_update.h may have such side-effect if something is wrong e.g. some property node's Clone() method or == operator is incomplete, or we miss checking some properties.
Project Member

Comment 11 by bugdroid1@chromium.org, May 2 2018

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

commit 556cf9a4d9636b05ec0062c65d212f5f2fe8c771
Author: Tien-Ren Chen <trchen@chromium.org>
Date: Wed May 02 01:37:34 2018

[Blink] Correctly derive context when no property changed

There was a mistake in PaintPropertyTreeBuilder::UpdateClipPathClip()
to early exit when properties didn't change. However in that case we
still have to update the context for descendants to inherit. This CL
removed the early exit.

BUG= 836127 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I38b229b86bd9c2106ac32bf54d520d39cb892e0c
Reviewed-on: https://chromium-review.googlesource.com/1038705
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Tien-Ren Chen <trchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555275}
[modify] https://crrev.com/556cf9a4d9636b05ec0062c65d212f5f2fe8c771/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc
[modify] https://crrev.com/556cf9a4d9636b05ec0062c65d212f5f2fe8c771/third_party/blink/renderer/core/paint/paint_property_tree_builder_test.cc

*** Bulk Edit ***
M67 Stable promotion is coming soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. 

If fix is already merged to M67 and nothing else is pending, pls mark the bug as fixed. Thank you.
Labels: Merge-Request-67
NextAction: 2018-05-03
CL listed at #11 is not in canary yet. Pls update the bug with canary result tomorrow. Thank you.
Project Member

Comment 15 by sheriffbot@chromium.org, May 3 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: TE-Verified-M68 TE-Verified-68.0.3418.0
Able to reproduce this issue on Windows 10, Ubuntu 14.04 and Mac OS 10.12.6 on the reported version 68.0.3405.0 and the issue is fixed on the latest Canary 68.0.3418.0 as per the original comment.

On navigating to the given codepen link and clicking on 'Change Fill' button, can observe that only the color of the rounded rectangle is changed.
Attached is the screen shot for reference.

Hence adding TE verified labels as the fix is working as intended.

Thanks..
836127-M68.png
84.6 KB View Download
No longer able to reproduce this issue on Windows 7 with Canary 68.0.3418.0 

Also, with Canary 68.0.3418.0 the issue does not appear anymore in the original scenario, from which the test case was derived.
The NextAction date has arrived: 2018-05-03
Labels: -Merge-Review-67 Merge-Approved-67
Approving merge to M67 branch 3396 based on comment #16 and #17. Please merge ASAP.

Also pls mark the bug as fixed after the merge if nothing else is pending. Thank you.
Project Member

Comment 20 by bugdroid1@chromium.org, May 3 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c2affae07d9ad7dac3aa209238a61e830de13926

commit c2affae07d9ad7dac3aa209238a61e830de13926
Author: Tien-Ren Chen <trchen@chromium.org>
Date: Thu May 03 18:39:36 2018

[Blink] Correctly derive context when no property changed

There was a mistake in PaintPropertyTreeBuilder::UpdateClipPathClip()
to early exit when properties didn't change. However in that case we
still have to update the context for descendants to inherit. This CL
removed the early exit.

BUG= 836127 
TBR=trchen@chromium.org

(cherry picked from commit 556cf9a4d9636b05ec0062c65d212f5f2fe8c771)

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I38b229b86bd9c2106ac32bf54d520d39cb892e0c
Reviewed-on: https://chromium-review.googlesource.com/1038705
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Tien-Ren Chen <trchen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#555275}
Reviewed-on: https://chromium-review.googlesource.com/1042821
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#461}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/c2affae07d9ad7dac3aa209238a61e830de13926/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc
[modify] https://crrev.com/c2affae07d9ad7dac3aa209238a61e830de13926/third_party/blink/renderer/core/paint/paint_property_tree_builder_test.cc

Status: Fixed (was: Started)

Sign in to add a comment