Issue 191353002: [CSS Shapes] Omit polygon default fill-rule

Issue 191353002: [CSS Shapes] Omit polygon default fill-rule (Closed)

Created:
6 years, 9 months ago by rwlbuis

Modified:
6 years, 9 months ago

Reviewers:
leviw_travelin_and_unemployed, Bear Travis

CC:
blink-reviews, ed+blinkwatch_opera.com, shans, rjwright, alancutter (OOO until 2018), Mike Lawther (Google), rune+blink, dglazkov+blink, dstockwell, Timothy Loh, apavlov+blink_chromium.org, darktears, Steve Block, dino_apple.com, Eric Willigers, Bear Travis

Base URL:
https://chromium.googlesource.com/chromium/blink.git@master

Visibility:
Public.

More Reviews

Description

[CSS Shapes] Omit polygon default fill-rule Omit the default value (nonzero) when serializing the fill-rule. Adjust the tests to this behavior. BUG=350148 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169035

Patch Set 1 #

Patch Set 2 : Fix failing tests #

Total comments: 1

Patch Set 3 : Add COMPILE_ASSERT #

Created: 6 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+68 lines, -68 lines)			Patch
M	LayoutTests/css3/masking/clip-path-animation.html	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	LayoutTests/css3/masking/clip-path-animation-expected.txt	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	LayoutTests/fast/masking/parsing-clip-path-shape.html	View	1 2	1 chunk	+2 lines, -2 lines	0 comments	Download
M	LayoutTests/fast/masking/parsing-clip-path-shape-expected.txt	View	1 2	1 chunk	+2 lines, -2 lines	0 comments	Download
M	LayoutTests/fast/shapes/parsing/parsing-shape-inside-expected.txt	View	1 2	2 chunks	+20 lines, -20 lines	0 comments	Download
M	LayoutTests/fast/shapes/parsing/parsing-shape-lengths.html	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	LayoutTests/fast/shapes/parsing/parsing-shape-lengths-expected.txt	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	LayoutTests/fast/shapes/parsing/parsing-shape-outside-expected.txt	View	1 2	2 chunks	+20 lines, -20 lines	0 comments	Download
M	LayoutTests/fast/shapes/parsing/parsing-test-utils.js	View	1 2	1 chunk	+10 lines, -10 lines	0 comments	Download
M	LayoutTests/fast/shapes/shape-inside/shape-inside-animation.html	View		1 chunk	+1 line, -1 line	0 comments	Download
M	LayoutTests/fast/shapes/shape-inside/shape-inside-animation-expected.txt	View		1 chunk	+1 line, -1 line	0 comments	Download
M	LayoutTests/transitions/shape-transition.html	View	1	1 chunk	+2 lines, -2 lines	0 comments	Download
M	LayoutTests/transitions/shape-transition-expected.txt	View	1	1 chunk	+2 lines, -2 lines	0 comments	Download
M	Source/core/css/CSSBasicShapes.cpp	View	1 2	1 chunk	+2 lines, -2 lines	0 comments	Download

Messages

Total messages: 12 (0 generated)

Expand Messages | Collapse Messages

Bear Travis

lgtm, fwiw. The only odd bit is that the string builder may over-reserve capacity.

6 years, 9 months ago (2014-03-07 19:10:19 UTC) #1

rwlbuis

This is a spec thing again, the specifics are in the (imported) bug :)

6 years, 9 months ago (2014-03-11 22:54:43 UTC) #2

leviw_travelin_and_unemployed

lgtm. https://codereview.chromium.org/191353002/diff/20001/Source/core/css/CSSBasicShapes.cpp File Source/core/css/CSSBasicShapes.cpp (left): https://codereview.chromium.org/191353002/diff/20001/Source/core/css/CSSBasicShapes.cpp#oldcode296 Source/core/css/CSSBasicShapes.cpp:296: COMPILE_ASSERT(sizeof(evenOddOpening) == sizeof(nonZeroOpening), polygon_string_openings_have_same_length); Would it make sense ...

6 years, 9 months ago (2014-03-11 23:07:12 UTC) #3

rwlbuis

On 2014/03/11 23:07:12, Levi wrote: > lgtm. > > https://codereview.chromium.org/191353002/diff/20001/Source/core/css/CSSBasicShapes.cpp > File Source/core/css/CSSBasicShapes.cpp (left): > ...

6 years, 9 months ago (2014-03-12 14:44:33 UTC) #4