Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1042)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 839903003: [CSS Masking][CSS Shapes] Large corner radii use with inset() clip-path are not properly constrained (Closed)

Created:
5 years, 11 months ago by rwlbuis
Modified:
5 years, 11 months ago
CC:
blink-reviews, blink-reviews-rendering, Dominik Röttsches, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, zoltan1
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

[CSS Masking][CSS Shapes] Large corner radii use with inset() clip-path are not properly constrained Constrain large corner radii for inset() clip-path in the same way that border-radii are constrained. This is a merge of http://trac.webkit.org/changeset/178015 by Bem Jones-Bey <bjonesbe@adobe.com>;. BUG=449638 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=188734

Patch Set 1 #

Patch Set 2 : Add test expectations #

Patch Set 3 : Adds NeedsRebaseline #

Total comments: 1

Patch Set 4 : Adjust test #

Total comments: 3

Patch Set 5 : Fix issues #

Patch Set 6 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -10 lines) Patch
M LayoutTests/TestExpectations View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
A LayoutTests/fast/masking/clip-path-inset-large-radii.html View 1 2 3 4 5 1 chunk +15 lines, -0 lines 0 comments Download
A LayoutTests/platform/linux/fast/masking/clip-path-inset-large-radii-expected.png View 1 2 3 4 5 Binary file 0 comments Download
A LayoutTests/platform/linux/fast/masking/clip-path-inset-large-radii-expected.txt View 1 2 3 4 5 1 chunk +21 lines, -0 lines 0 comments Download
M Source/core/rendering/style/BasicShapes.cpp View 1 2 3 4 2 chunks +8 lines, -10 lines 0 comments Download

Messages

Total messages: 28 (10 generated)
rwlbuis
PTAL. As discussed, we unfortunately can't rely on the original reftest because of subtle pixel ...
5 years, 11 months ago (2015-01-16 21:45:41 UTC) #2
Bem Jones-Bey (adobe)
Other than the nit below, lgtm. https://codereview.chromium.org/839903003/diff/40001/LayoutTests/fast/masking/clip-path-inset-large-radii.html File LayoutTests/fast/masking/clip-path-inset-large-radii.html (right): https://codereview.chromium.org/839903003/diff/40001/LayoutTests/fast/masking/clip-path-inset-large-radii.html#newcode11 LayoutTests/fast/masking/clip-path-inset-large-radii.html:11: <p><a href="http://webkit.org/b/140127">Bug 140127</a> ...
5 years, 11 months ago (2015-01-16 22:36:06 UTC) #3
rwlbuis
PTAL @leviw
5 years, 11 months ago (2015-01-16 23:34:37 UTC) #5
Bem Jones-Bey (adobe)
https://codereview.chromium.org/839903003/diff/60001/LayoutTests/fast/masking/clip-path-inset-large-radii.html File LayoutTests/fast/masking/clip-path-inset-large-radii.html (right): https://codereview.chromium.org/839903003/diff/60001/LayoutTests/fast/masking/clip-path-inset-large-radii.html#newcode11 LayoutTests/fast/masking/clip-path-inset-large-radii.html:11: <p><a href="crbug.com/">Issue 449638</a> - [CSS Masking][CSS Shapes] Large corner ...
5 years, 11 months ago (2015-01-16 23:42:57 UTC) #6
leviw_travelin_and_unemployed
https://codereview.chromium.org/839903003/diff/60001/LayoutTests/fast/masking/clip-path-inset-large-radii.html File LayoutTests/fast/masking/clip-path-inset-large-radii.html (right): https://codereview.chromium.org/839903003/diff/60001/LayoutTests/fast/masking/clip-path-inset-large-radii.html#newcode1 LayoutTests/fast/masking/clip-path-inset-large-radii.html:1: <!DOCTYPE html> Could this be a ref test instead ...
5 years, 11 months ago (2015-01-17 00:18:22 UTC) #7
rwlbuis
On 2015/01/16 23:42:57, Bem Jones-Bey (adobe) wrote: > https://codereview.chromium.org/839903003/diff/60001/LayoutTests/fast/masking/clip-path-inset-large-radii.html > File LayoutTests/fast/masking/clip-path-inset-large-radii.html (right): > > ...
5 years, 11 months ago (2015-01-19 19:22:05 UTC) #9
rwlbuis
On 2015/01/17 00:18:22, leviw wrote: > https://codereview.chromium.org/839903003/diff/60001/LayoutTests/fast/masking/clip-path-inset-large-radii.html > File LayoutTests/fast/masking/clip-path-inset-large-radii.html (right): > > https://codereview.chromium.org/839903003/diff/60001/LayoutTests/fast/masking/clip-path-inset-large-radii.html#newcode1 > ...
5 years, 11 months ago (2015-01-19 19:31:11 UTC) #10
Bem Jones-Bey (adobe)
On 2015/01/19 19:31:11, rwlbuis wrote: > On 2015/01/17 00:18:22, leviw wrote: > > > https://codereview.chromium.org/839903003/diff/60001/LayoutTests/fast/masking/clip-path-inset-large-radii.html ...
5 years, 11 months ago (2015-01-19 22:17:08 UTC) #11
leviw_travelin_and_unemployed
On 2015/01/19 at 22:17:08, bjonesbe wrote: > On 2015/01/19 19:31:11, rwlbuis wrote: > > On ...
5 years, 11 months ago (2015-01-20 19:53:11 UTC) #12
rwlbuis
On 2015/01/20 19:53:11, leviw wrote: > On 2015/01/19 at 22:17:08, bjonesbe wrote: > > It's ...
5 years, 11 months ago (2015-01-20 20:06:08 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/839903003/120001
5 years, 11 months ago (2015-01-20 20:49:29 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: win_gpu_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered_tests/builds/96534)
5 years, 11 months ago (2015-01-20 23:00:38 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/839903003/120001
5 years, 11 months ago (2015-01-20 23:11:38 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: win_gpu_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered_tests/builds/96589)
5 years, 11 months ago (2015-01-21 01:13:21 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/839903003/120001
5 years, 11 months ago (2015-01-21 01:15:55 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: win_gpu_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered_tests/builds/96744)
5 years, 11 months ago (2015-01-21 05:09:36 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/839903003/120001
5 years, 11 months ago (2015-01-21 11:44:21 UTC) #27
commit-bot: I haz the power
5 years, 11 months ago (2015-01-21 12:44:21 UTC) #28
Message was sent while issue was closed. 
Committed patchset #6 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=188734

Powered by Google App Engine
This is Rietveld 408576698