Bug 127802 - [CSS Shapes] Serialize basic shape positions
Summary: [CSS Shapes] Serialize basic shape positions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Bear Travis
URL:
Keywords:
Depends on: 129404 129700
Blocks: 124173
  Show dependency treegraph
 
Reported: 2014-01-28 14:30 PST by Rob Buis
Modified: 2014-03-07 15:39 PST (History)
9 users (show)

See Also:


Attachments
Patch (38.13 KB, patch)
2014-01-28 14:35 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (40.42 KB, patch)
2014-01-29 08:07 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (13.06 KB, patch)
2014-01-31 14:33 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (26.48 KB, patch)
2014-02-04 11:47 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (30.96 KB, patch)
2014-02-05 08:36 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (556.00 KB, application/zip)
2014-02-05 09:18 PST, Build Bot
no flags Details
Patch (21.50 KB, patch)
2014-02-05 11:28 PST, Rob Buis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.