Bug 127785

Summary: [CSS Shapes] Adjust inset sizing syntax to the latest spec
Product: WebKit Reporter: Zoltan Horvath <zoltan>
Component: CSSAssignee: Zoltan Horvath <zoltan>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, gyuyoung.kim, macpherson, menard
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 124173    
Attachments:
Description Flags
Patch
none
Patch
none
Patch - linear parsing logic and less variable assignments
none
Patch - linear parsing logic and less variable assignments none

Zoltan Horvath
Reported 2014-01-28 11:31:02 PST
According to the latest CSS Shapes specification [1], the width arguments of inset should follow the syntax of the margin shorthand, which let us set all four insets with one, two or four values. This patch updates the behavior and updates the affected tests. [1] http://dev.w3.org/csswg/css-shapes/#funcdef-inset
Attachments
Patch (43.38 KB, patch)
2014-01-28 11:35 PST, Zoltan Horvath 		no flags
DetailsFormatted DiffDiff
Patch (44.03 KB, patch)
2014-01-28 14:01 PST, Zoltan Horvath 		no flags
DetailsFormatted DiffDiff
Patch - linear parsing logic and less variable assignments (43.35 KB, patch)
2014-01-28 15:20 PST, Zoltan Horvath 		no flags
DetailsFormatted DiffDiff
Patch - linear parsing logic and less variable assignments (43.08 KB, patch)
2014-01-28 16:10 PST, Zoltan Horvath 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Zoltan Horvath
Comment 1 2014-01-28 11:35:23 PST
Created attachment 222469 [details] Patch
Bem Jones-Bey
Comment 2 2014-01-28 11:47:07 PST
Comment on attachment 222469 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=222469&action=review > Source/WebCore/css/CSSParser.cpp:5409 > + if (widthArguments.size() == 1) { I think a switch would be more readable here. > Source/WebCore/css/CSSParser.cpp:5422 > + shape->setBottom(widthArguments[2]); You need to set Left here, and double check your tests, if this passed, you missed a test.
Bem Jones-Bey
Comment 3 2014-01-28 11:48:27 PST
Comment on attachment 222469 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=222469&action=review > LayoutTests/fast/shapes/parsing/parsing-test-utils.js:19 > ["inset(10px 9px 8px)", "inset(10px 9px 8px)", "inset(10px 9px 8px 0px round 0px 0px 0px 0px / 0px 0px 0px 0px)"], Here's the wrong test!
Zoltan Horvath
Comment 4 2014-01-28 14:01:32 PST
Created attachment 222483 [details] Patch (In reply to comment #2) > (From update of attachment 222469 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=222469&action=review > > > Source/WebCore/css/CSSParser.cpp:5409 > > + if (widthArguments.size() == 1) { > > I think a switch would be more readable here. I modified to use switch. > > Source/WebCore/css/CSSParser.cpp:5422 > > + shape->setBottom(widthArguments[2]); > > You need to set Left here, and double check your tests, if this passed, you missed a test. That behavior was the expected in my mind. :( I added the corrected case and I modified the affected test.
Zoltan Horvath
Comment 5 2014-01-28 15:20:54 PST
Created attachment 222505 [details] Patch - linear parsing logic and less variable assignments
Zoltan Horvath
Comment 6 2014-01-28 16:10:15 PST
Created attachment 222526 [details] Patch - linear parsing logic and less variable assignments
Bem Jones-Bey
Comment 7 2014-01-28 16:23:09 PST
Comment on attachment 222526 [details] Patch - linear parsing logic and less variable assignments r=me
WebKit Commit Bot
Comment 8 2014-01-28 17:17:43 PST
Comment on attachment 222526 [details] Patch - linear parsing logic and less variable assignments Clearing flags on attachment: 222526 Committed r162989: <http://trac.webkit.org/changeset/162989>
WebKit Commit Bot
Comment 9 2014-01-28 17:17:45 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.