Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(29)

Issue 2799793002: Implement color stop position syntax from CSS Image Values 4 (Closed)

Created:
2 years, 8 months ago by f(malita)
Modified:
2 years, 8 months ago
Reviewers:
Stephen Chennney, fs
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, chromium-reviews, dglazkov+blink, reed1, rwlbuis
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Implement color stop position syntax from CSS Image Values 4 CSS Image Values 4 allows specifying up to two positions per color stop, e.g.: linear-gradient(45deg, black 0% 50%, white 50% 100%) This is equivalent to repeating the color at the given positions: linear-gradient(45deg, black 0%, black 50%, white 50%, white 100%) (https://drafts.csswg.org/css-images-4/#color-stop-syntax) Add support for the new syntax, behind a runtime flag. BUG=707047

Patch Set 1 #

Patch Set 2 : accurate computed value #

Patch Set 3 : no repeating hints #

Patch Set 4 : hide behind RTEF #

Total comments: 5

Patch Set 5 : review #

Patch Set 6 : greedy color repeat serializer + more tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+71 lines, -2 lines) Patch
M third_party/WebKit/LayoutTests/fast/gradients/conic-gradient-parsing.html View 1 2 3 4 5 2 chunks +12 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/gradients/unprefixed-gradient-parsing.html View 1 2 2 chunks +16 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/gradients/unprefixed-gradient-parsing-expected.txt View 1 2 2 chunks +14 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSGradientValue.cpp View 1 2 3 4 5 1 chunk +14 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp View 1 2 3 4 1 chunk +11 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5 View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 35 (23 generated)
f(malita)
Per offline conversation with rbyers@, it should be OK to land this behind a flag ...
2 years, 8 months ago (2017-04-06 19:12:35 UTC) #13
fs
LGTM w/ some thoughts on additional tests, and some REF feature "wrapping" pondering. https://codereview.chromium.org/2799793002/diff/60001/third_party/WebKit/Source/core/css/CSSGradientValue.cpp File ...
2 years, 8 months ago (2017-04-06 19:58:48 UTC) #16
f(malita)
https://codereview.chromium.org/2799793002/diff/60001/third_party/WebKit/Source/core/css/CSSGradientValue.cpp File third_party/WebKit/Source/core/css/CSSGradientValue.cpp (right): https://codereview.chromium.org/2799793002/diff/60001/third_party/WebKit/Source/core/css/CSSGradientValue.cpp#newcode1081 third_party/WebKit/Source/core/css/CSSGradientValue.cpp:1081: // At most two consecutive stops can share the ...
2 years, 8 months ago (2017-04-06 20:29:43 UTC) #18
fs
https://codereview.chromium.org/2799793002/diff/60001/third_party/WebKit/Source/core/css/CSSGradientValue.cpp File third_party/WebKit/Source/core/css/CSSGradientValue.cpp (right): https://codereview.chromium.org/2799793002/diff/60001/third_party/WebKit/Source/core/css/CSSGradientValue.cpp#newcode1081 third_party/WebKit/Source/core/css/CSSGradientValue.cpp:1081: // At most two consecutive stops can share the ...
2 years, 8 months ago (2017-04-06 20:44:36 UTC) #20
f(malita)
On 2017/04/06 20:44:36, fs wrote: > Anyway, we can keep it as-is (this is > ...
2 years, 8 months ago (2017-04-06 21:01:04 UTC) #21
fs
On 2017/04/06 at 21:01:04, fmalita wrote: > On 2017/04/06 20:44:36, fs wrote: > > Anyway, ...
2 years, 8 months ago (2017-04-06 21:05:49 UTC) #22
Rick Byers
On 2017/04/06 19:12:35, f(malita) wrote: > Per offline conversation with rbyers@, it should be OK ...
2 years, 8 months ago (2017-04-06 21:35:05 UTC) #23
f(malita)
On 2017/04/06 21:01:04, f(malita) wrote: > > Since we're on the fence, I think compressing ...
2 years, 8 months ago (2017-04-06 22:10:00 UTC) #26
fs
On 2017/04/06 at 22:10:00, fmalita wrote: > On 2017/04/06 21:01:04, f(malita) wrote: > > > ...
2 years, 8 months ago (2017-04-07 07:41:55 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2799793002/100001
2 years, 8 months ago (2017-04-07 11:57:09 UTC) #31
commit-bot: I haz the power
Prior attempt to commit was detected, but we were not able to check whether the ...
2 years, 8 months ago (2017-04-07 12:07:36 UTC) #34
f(malita)
2 years, 8 months ago (2017-04-07 13:13:19 UTC) #35
On 2017/04/07 12:07:36, commit-bot: I haz the power wrote:
> Prior attempt to commit was detected, but we were not able to check whether
the
> issue was successfully committed. Please check Git history manually and
re-check
> CQ or close this issue as needed.

Yes, confused commit-bot -- the issue was indeed committed:
https://chromium.googlesource.com/chromium/src/+/546c77f5d4ce77af661664954f0a....

Powered by Google App Engine
This is Rietveld 408576698