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

Issue 1219153003: Unify handling of <color> for SVG/non-SVG properties (Closed)

Created:
5 years, 5 months ago by fs
Modified:
5 years, 5 months ago
Reviewers:
Timothy Loh
CC:
blink-reviews, blink-reviews-style_chromium.org, blink-reviews-css, dglazkov+blink, apavlov+blink_chromium.org, darktears, rwlbuis
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Unify handling of <color> for SVG/non-SVG properties Stop resolving system colors and "basic" named colors at parse time, and instead resolve them when resolving style. Implementation-wise this means getting rid of CSSPropertyParser::parseSVGColor in favor of parseColor, and similarly get rid of convertSVGColor in favor of convertColor on the style resolver side. Changes behavior for "basic" color names and system colors (as well as some proprietary/internal keywords). New behavior matches Gecko. BUG=370830, 505410 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=198199

Patch Set 1 #

Patch Set 2 : More expectation changes. #

Patch Set 3 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -59 lines) Patch
M LayoutTests/fast/css/parse-color-int-or-percent-crash.html View 1 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/inspector/animation/animation-timeline-expected.txt View 1 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/svg/dom/rgb-color-parser-expected.txt View 2 chunks +4 lines, -4 lines 0 comments Download
M Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl View 1 chunk +17 lines, -19 lines 0 comments Download
M Source/core/css/CSSProperties.in View 1 2 3 chunks +3 lines, -3 lines 0 comments Download
M Source/core/css/parser/CSSPropertyParser.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/css/parser/CSSPropertyParser.cpp View 4 chunks +3 lines, -17 lines 0 comments Download
M Source/core/css/resolver/StyleBuilderConverter.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/css/resolver/StyleBuilderConverter.cpp View 1 chunk +0 lines, -9 lines 0 comments Download

Messages

Total messages: 6 (2 generated)
fs
5 years, 5 months ago (2015-07-01 11:56:55 UTC) #2
Timothy Loh
On 2015/07/01 11:56:55, fs wrote: lgtm
5 years, 5 months ago (2015-07-02 01:44:54 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1219153003/40001
5 years, 5 months ago (2015-07-02 08:39:45 UTC) #5
commit-bot: I haz the power
5 years, 5 months ago (2015-07-02 08:43:28 UTC) #6
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=198199

Powered by Google App Engine
This is Rietveld 408576698