Issue 1177303004: Updated SVGListPropertyHelper as per SVG2 Spec

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+1075 lines, -789 lines)			Patch
M	LayoutTests/TestExpectations	View	1 2 3 4	1 chunk	+10 lines, -0 lines	0 comments	Download
M	LayoutTests/platform/linux/svg/dom/SVGLengthList-appendItem-expected.png	View	1 2	Binary file		0 comments	Download
M	LayoutTests/platform/linux/svg/dom/SVGLengthList-initialize-expected.png	View	1 2	Binary file		0 comments	Download
M	LayoutTests/platform/linux/svg/dom/SVGLengthList-insertItemBefore-expected.png	View	1 2	Binary file		0 comments	Download
M	LayoutTests/platform/linux/svg/dom/SVGLengthList-removeItem-expected.png	View	1 2	Binary file		0 comments	Download
M	LayoutTests/platform/linux/svg/dom/SVGLengthList-replaceItem-expected.png	View	1 2	Binary file		0 comments	Download
M	LayoutTests/platform/linux/svg/dom/SVGPathSegList-appendItem-expected.png	View	1 2 3 4	Binary file		0 comments	Download
M	LayoutTests/platform/linux/svg/dom/SVGPathSegList-clear-and-initialize-expected.png	View	1 2	Binary file		0 comments	Download
M	LayoutTests/platform/linux/svg/dom/SVGPathSegList-insertItemBefore-expected.png	View	1 2 3 4	Binary file		0 comments	Download
M	LayoutTests/platform/linux/svg/dom/SVGPathSegList-replaceItem-expected.png	View	1 2	Binary file		0 comments	Download
M	LayoutTests/svg/dom/SVGLengthList-appendItem.xhtml	View	1 2	1 chunk	+49 lines, -41 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGLengthList-appendItem-expected.txt	View	1 2	1 chunk	+50 lines, -42 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGLengthList-basics.xhtml	View	1 2	3 chunks	+30 lines, -19 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGLengthList-basics-expected.txt	View	1 2	3 chunks	+30 lines, -20 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGLengthList-initialize.xhtml	View	1 2 3 4	1 chunk	+10 lines, -10 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGLengthList-initialize-expected.txt	View	1 2	1 chunk	+7 lines, -7 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGLengthList-insertItemBefore.xhtml	View	1 2 3 4	1 chunk	+16 lines, -13 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGLengthList-insertItemBefore-expected.txt	View	1 2	1 chunk	+14 lines, -11 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGLengthList-removeItem.xhtml	View	1 2	1 chunk	+18 lines, -104 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGLengthList-removeItem-expected.txt	View	1 2	1 chunk	+13 lines, -88 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGLengthList-replaceItem.xhtml	View	1 2 3 4	5 chunks	+33 lines, -14 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGLengthList-replaceItem-expected.txt	View	1 2	4 chunks	+29 lines, -13 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGNumberList-basics.xhtml	View	1 2	1 chunk	+22 lines, -12 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGNumberList-basics-expected.txt	View	1 2	1 chunk	+22 lines, -12 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGPathSegList-appendItem.xhtml	View	1 2 3	3 chunks	+30 lines, -5 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGPathSegList-appendItem-expected.txt	View	1 2 3	3 chunks	+27 lines, -4 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGPathSegList-clear-and-initialize.xhtml	View	1 2	1 chunk	+9 lines, -6 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGPathSegList-clear-and-initialize-expected.txt	View	1 2	1 chunk	+9 lines, -6 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGPathSegList-insertItemBefore.xhtml	View	1 2 3 4	2 chunks	+24 lines, -1 line	0 comments	Download
M	LayoutTests/svg/dom/SVGPathSegList-insertItemBefore-expected.txt	View	1 2 3 4	2 chunks	+23 lines, -1 line	0 comments	Download
M	LayoutTests/svg/dom/SVGPathSegList-replaceItem.xhtml	View	1 2	2 chunks	+12 lines, -6 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGPathSegList-replaceItem-expected.txt	View	1 2	2 chunks	+10 lines, -6 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGPointList-basics.xhtml	View	1 2	2 chunks	+60 lines, -33 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGPointList-basics-expected.txt	View	1 2	2 chunks	+60 lines, -33 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGTransformList-basics.xhtml	View	1 2 3 4	3 chunks	+115 lines, -127 lines	0 comments	Download
M	LayoutTests/svg/dom/SVGTransformList-basics-expected.txt	View	1 2 3 4	1 chunk	+107 lines, -97 lines	0 comments	Download
A	LayoutTests/svg/dom/SVGTransformList-with-existing-item.html	View	1 2 3 4	1 chunk	+70 lines, -0 lines	0 comments	Download
A	LayoutTests/svg/dom/SVGTransformList-with-existing-item-expected.txt	View	1 2 3 4	1 chunk	+36 lines, -0 lines	0 comments	Download
A	LayoutTests/svg/dom/resources/SVGTransformList-helper.js	View	1 2 3 4 5 6	1 chunk	+24 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSeg.h	View	1 2	1 chunk	+1 line, -1 line	0 comments	Download
M	Source/core/svg/SVGPathSegArcAbs.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegArcRel.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegClosePath.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegCurvetoCubicAbs.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegCurvetoCubicRel.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegCurvetoCubicSmoothAbs.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegCurvetoCubicSmoothRel.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegCurvetoQuadraticAbs.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegCurvetoQuadraticRel.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegCurvetoQuadraticSmoothAbs.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegCurvetoQuadraticSmoothRel.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegLinetoAbs.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegLinetoHorizontalAbs.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegLinetoHorizontalRel.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegLinetoRel.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegLinetoVerticalAbs.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegLinetoVerticalRel.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegListTearOff.h	View	1 2 3 4	1 chunk	+7 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegMovetoAbs.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/SVGPathSegMovetoRel.h	View	1 2 3 4 5	1 chunk	+5 lines, -0 lines	0 comments	Download
M	Source/core/svg/properties/SVGListPropertyHelper.h	View	1 2 3 4	6 chunks	+0 lines, -55 lines	0 comments	Download
M	Source/core/svg/properties/SVGListPropertyTearOffHelper.h	View	1 2 3 4	1 chunk	+3 lines, -2 lines	0 comments	Download

Messages

Total messages: 28 (10 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

rwlbuis

On 2015/06/12 07:24:42, Shanmuga Pandi wrote: > Please review this. > Thanks Peer review looks ...

5 years, 6 months ago (2015-06-12 16:14:41 UTC) #3

pdr.

Overall this looks pretty good, just nits below. https://codereview.chromium.org/1177303004/diff/1/LayoutTests/svg/dom/SVGTransformList-more.xhtml File LayoutTests/svg/dom/SVGTransformList-more.xhtml (right): https://codereview.chromium.org/1177303004/diff/1/LayoutTests/svg/dom/SVGTransformList-more.xhtml#newcode1 LayoutTests/svg/dom/SVGTransformList-more.xhtml:1: <html ...

5 years, 6 months ago (2015-06-12 20:23:24 UTC) #6

On 2015/06/12 16:14:41, rwlbuis wrote: ... > Do you think this is the desired behavior ...

5 years, 6 months ago (2015-06-12 22:54:36 UTC) #7

Shanmuga Pandi

https://codereview.chromium.org/1177303004/diff/1/LayoutTests/svg/dom/SVGTransformList-more.xhtml File LayoutTests/svg/dom/SVGTransformList-more.xhtml (right): https://codereview.chromium.org/1177303004/diff/1/LayoutTests/svg/dom/SVGTransformList-more.xhtml#newcode1 LayoutTests/svg/dom/SVGTransformList-more.xhtml:1: <html xmlns="http://www.w3.org/1999/xhtml"> On 2015/06/12 20:23:24, pdr wrote: > Nit: ...

5 years, 6 months ago (2015-06-13 07:01:32 UTC) #8

https://codereview.chromium.org/1177303004/diff/1/LayoutTests/svg/dom/SVGTran...
File LayoutTests/svg/dom/SVGTransformList-more.xhtml (right):

https://codereview.chromium.org/1177303004/diff/1/LayoutTests/svg/dom/SVGTran...
LayoutTests/svg/dom/SVGTransformList-more.xhtml:1: <html
xmlns="http://www.w3.org/1999/xhtml">
On 2015/06/12 20:23:24, pdr wrote:
> Nit: Please use <!DOCTYPE HTML> as the first line.
> 
> We can probably remove window.enablePixelTesting as well.

Done.

https://codereview.chromium.org/1177303004/diff/1/LayoutTests/svg/dom/SVGTran...
LayoutTests/svg/dom/SVGTransformList-more.xhtml:31: function
dumpTransform(transform) {
On 2015/06/12 20:23:24, pdr wrote:
> Instead of copying these, lets move them to a common javascript file and
include
> them in both tests.

Done.

https://codereview.chromium.org/1177303004/diff/1/LayoutTests/svg/dom/SVGTran...
LayoutTests/svg/dom/SVGTransformList-more.xhtml:49: var transT =
svg.createSVGTransform();
On 2015/06/12 20:23:24, pdr wrote:
> Nit: no need to abbreviate, lets just call this translate and the scale one
> "scale".

Done.

https://codereview.chromium.org/1177303004/diff/1/LayoutTests/svg/dom/SVGTran...
LayoutTests/svg/dom/SVGTransformList-more.xhtml:66:
shouldBeEqualToString("dumpTransform(circle2.transform.baseVal.getItem(1))",
"type=SVG_TRANSFORM_TRANSLATE matrix=[1.0 0.0 0.0 1.0 100.0 100.0]");
On 2015/06/12 20:23:24, pdr wrote:
> Why is this 1.0 0.0 0.0 1.0 100.0 100.0 instead of 1.0 0.0 0.0 1.0 200.0 200.0
?

circle1.transform.baseVal.appendItem(transT);	
circle2.transform.baseVal.appendItem(transT);

With the above, circle2 will have new SVGTransform object with the same values
of transT, since transT already part of circle1 list.
So modifying trasT, will not affect circle2.

https://codereview.chromium.org/1177303004/diff/1/LayoutTests/svg/dom/SVGTran...
LayoutTests/svg/dom/SVGTransformList-more.xhtml:81:
shouldBeEqualToString("dumpTransform(circle1.transform.baseVal.getItem(2))",
"type=SVG_TRANSFORM_SCALE matrix=[3.0 0.0 0.0 3.0 0.0 0.0]");
On 2015/06/12 20:23:24, pdr wrote:
> Was this supposed to be circle2? (here, and line 90)

sorry. type error. Corrected

Shanmuga Pandi

On 2015/06/12 22:54:36, fs wrote: > On 2015/06/12 16:14:41, rwlbuis wrote: > ... > > ...

5 years, 6 months ago (2015-06-13 07:05:18 UTC) #9

On 2015/06/13 07:05:18, Shanmuga Pandi wrote: > On 2015/06/12 22:54:36, fs wrote: > > On ...

5 years, 6 months ago (2015-06-15 13:36:54 UTC) #12

Shanmuga Pandi

On 2015/06/15 13:36:54, fs wrote: > On 2015/06/13 07:05:18, Shanmuga Pandi wrote: > > On ...

5 years, 6 months ago (2015-06-15 14:02:12 UTC) #13

Shanmuga Pandi

Modified the code as per comments. Please review it. Thanks

5 years, 6 months ago (2015-06-17 13:06:19 UTC) #14

+kouhei, being the tear-off expert. https://codereview.chromium.org/1177303004/diff/60001/LayoutTests/TestExpectations File LayoutTests/TestExpectations (right): https://codereview.chromium.org/1177303004/diff/60001/LayoutTests/TestExpectations#newcode705 LayoutTests/TestExpectations:705: crbug.com/497196 svg/dom/SVGLengthList-appendItem.xhtml [ NeedsRebaseline ...

5 years, 6 months ago (2015-06-17 16:02:05 UTC) #16

Shanmuga Pandi

https://codereview.chromium.org/1177303004/diff/60001/LayoutTests/TestExpectations File LayoutTests/TestExpectations (right): https://codereview.chromium.org/1177303004/diff/60001/LayoutTests/TestExpectations#newcode705 LayoutTests/TestExpectations:705: crbug.com/497196 svg/dom/SVGLengthList-appendItem.xhtml [ NeedsRebaseline ] On 2015/06/17 16:02:04, fs ...

5 years, 6 months ago (2015-06-18 11:30:45 UTC) #17

lgtm https://codereview.chromium.org/1177303004/diff/60001/LayoutTests/TestExpectations File LayoutTests/TestExpectations (right): https://codereview.chromium.org/1177303004/diff/60001/LayoutTests/TestExpectations#newcode705 LayoutTests/TestExpectations:705: crbug.com/497196 svg/dom/SVGLengthList-appendItem.xhtml [ NeedsRebaseline ] On 2015/06/18 11:30:45, ...

5 years, 6 months ago (2015-06-18 12:57:22 UTC) #18

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1177303004/100001

5 years, 6 months ago (2015-06-18 13:20:03 UTC) #21

commit-bot: I haz the power

Try jobs failed on following builders: blink_presubmit on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/35708)

5 years, 6 months ago (2015-06-18 13:27:13 UTC) #23

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1177303004/120001

5 years, 6 months ago (2015-06-18 13:50:36 UTC) #27

commit-bot: I haz the power

5 years, 6 months ago (2015-06-18 15:08:39 UTC) #28

Message was sent while issue was closed.

Committed patchset #7 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197359

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

Issue 1177303004: Updated SVGListPropertyHelper as per SVG2 Spec (Closed)

Description

Patch Set 1 #

Patch Set 2 : Align with review comments #

Patch Set 3 : Updated SVGListPropertyHelper interface #

Patch Set 4 : small nits #

Patch Set 5 : Align with review comments #

Patch Set 6 : fixed nits #

Patch Set 7 : fixed presubmit error #

Messages