Bug 155230

Summary: [css-grid] Fix placement for unknown named grid lines
Product: WebKit Reporter: Manuel Rego Casasnovas <rego>
Component: Layout and RenderingAssignee: Manuel Rego Casasnovas <rego>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, jfernandez, kondapallykalyan, rego, simon.fraser, svillar
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 153488    
Attachments:
Description Flags
Patch
none
Patch none

Manuel Rego Casasnovas
Reported 2016-03-09 04:49:14 PST
Placement for undefined named lines is not working as described in the spec: http://dev.w3.org/csswg/css-grid/#line-placement Mainly we're not following this statement from the spec: "If a name is given as a <custom-ident>, only lines with that name are counted. If not enough lines with that name exist, all lines in the implicit grid are assumed to have that name for the purpose of finding this position." Bug has been already fixed in Blink: https://bugs.chromium.org/p/chromium/issues/detail?id=442954 We need to port: https://codereview.chromium.org/1576993003
Attachments
Patch (70.77 KB, patch)
2016-03-09 08:20 PST, Manuel Rego Casasnovas 		no flags
DetailsFormatted DiffDiff
Patch (70.94 KB, patch)
2016-03-10 03:22 PST, Manuel Rego Casasnovas 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Manuel Rego Casasnovas
Comment 1 2016-03-09 08:20:44 PST
Created attachment 273430 [details] Patch
Sergio Villar Senin
Comment 2 2016-03-10 02:24:52 PST
Comment on attachment 273430 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=273430&action=review > Source/WebCore/rendering/style/GridResolvedPosition.cpp:129 > + return end + numberOfLines - 1; I think we can do this in a single line. > Source/WebCore/rendering/style/GridResolvedPosition.cpp:151 > + return start - numberOfLines + 1; Ditto > Source/WebCore/rendering/style/GridResolvedPosition.cpp:167 > + NamedGridLinesMap::const_iterator it = gridLinesNames.find(position.namedGridLine()); We can use auto for these two.
Manuel Rego Casasnovas
Comment 3 2016-03-10 03:22:49 PST
Created attachment 273556 [details] Patch
Manuel Rego Casasnovas
Comment 4 2016-03-10 03:23:30 PST
Thanks for the review, uploaded new patch for landing applying suggested changes.
WebKit Commit Bot
Comment 5 2016-03-10 04:15:23 PST
Comment on attachment 273556 [details] Patch Clearing flags on attachment: 273556 Committed r197930: <http://trac.webkit.org/changeset/197930>
WebKit Commit Bot
Comment 6 2016-03-10 04:15:28 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.