Bug 127802

Summary:

[CSS Shapes] Serialize basic shape positions

Product:

WebKit

Reporter:

Rob Buis <rwlbuis>

Component:

CSS

Assignee:

Bear Travis <betravis>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

betravis, buildbot, commit-queue, esprehn+autocc, glenn, gyuyoung.kim, macpherson, menard, rniwa

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

129404, 129700

Bug Blocks:

124173

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2	none
Patch	none

Description Rob Buis 2014-01-28 14:30:14 PST

Serialize basic shape positions.

Comment 1 Rob Buis 2014-01-28 14:35:50 PST

Created attachment 222494 [details]
Patch

Comment 2 Rob Buis 2014-01-29 08:07:18 PST

Created attachment 222573 [details]
Patch

Comment 3 Dean Jackson 2014-01-30 21:52:45 PST

Comment on attachment 222573 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=222573&action=review

> Source/WebCore/ChangeLog:8
> +         Serialize basic shape positions by trying to detect keywords and trying to be as close as possible to the original input.

Nit: extra whitespace char in indentation.

> Source/WebCore/ChangeLog:11
> +        * css/BasicShapeFunctions.cpp:
> +        (WebCore::valueForCenterCoordinate):

Could you describe what you did here?

Comment 4 Rob Buis 2014-01-31 13:34:40 PST

(In reply to comment #3)
> (From update of attachment 222573 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=222573&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +         Serialize basic shape positions by trying to detect keywords and trying to be as close as possible to the original input.
> 
> Nit: extra whitespace char in indentation.

Oops, will fix.

> > Source/WebCore/ChangeLog:11
> > +        * css/BasicShapeFunctions.cpp:
> > +        (WebCore::valueForCenterCoordinate):
> 
> Could you describe what you did here?

Will try to...
So thanks for the review, but I'll have to change the patch slightly since the spec for this changed earlier this week :) New patch coming up...

Comment 5 Rob Buis 2014-01-31 14:33:13 PST

Created attachment 222853 [details]
Patch

Comment 6 Rob Buis 2014-01-31 15:05:15 PST

Comment on attachment 222853 [details]
Patch

From the feedback I got this is the right way to go. However there are some more tests possible to add so I'll remove the review flag for now.

Comment 7 Rob Buis 2014-02-04 11:47:08 PST

Created attachment 223149 [details]
Patch

Comment 8 Rob Buis 2014-02-05 08:36:23 PST

Created attachment 223240 [details]
Patch

Comment 9 Build Bot 2014-02-05 09:18:16 PST

Comment on attachment 223240 [details]
Patch

Attachment 223240 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5596259373023232

New failing tests:
svg/transforms/transform-origin-css-property.xhtml

Comment 10 Build Bot 2014-02-05 09:18:18 PST

Created attachment 223244 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

Comment 11 Rob Buis 2014-02-05 11:28:13 PST

Created attachment 223250 [details]
Patch

Comment 12 Bear Travis 2014-02-26 15:20:48 PST

I will be taking a look at this.

Comment 13 Rob Buis 2014-02-27 13:33:04 PST

Comment on attachment 223250 [details]
Patch

Clearing review flag, Bear is taking over.

Comment 14 Bear Travis 2014-03-07 15:39:53 PST

Closing, as the two dependent issues have been fixed.