Bug 132122

Summary: [CSS Grid Layout] Implementation of the "grid" shorthand.
Product: WebKit Reporter: Javier Fernandez <jfernandez>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, esprehn+autocc, glenn, gyuyoung.kim, jfernandez, kling, macpherson, menard, rego, svillar, syoichi
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 128980    
Bug Blocks: 127987    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Javier Fernandez 2014-04-24 03:03:08 PDT
The grid property is a shorthand that sets all of the explicit grid properties (grid-template-rows, grid-template-columns, and grid-template-areas) as well as all the implicit grid properties (grid-auto-rows, grid-auto-columns, and grid-auto-flow) in a single declaration.
Comment 1 Javier Fernandez 2014-04-24 07:33:49 PDT
Created attachment 230078 [details]
Patch
Comment 2 Darin Adler 2014-04-24 07:57:52 PDT
Comment on attachment 230078 [details]
Patch

EWS was unable to apply the patch. I suggest rebasing and uploading a new one.
Comment 3 Javier Fernandez 2014-04-24 10:19:01 PDT
(In reply to comment #2)
> (From update of attachment 230078 [details])
> EWS was unable to apply the patch. I suggest rebasing and uploading a new one.

The problem is that it highly depends on bug #12898, so it'd be great if you or someone else could review it.
Comment 4 Javier Fernandez 2014-04-24 10:22:48 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 230078 [details] [details])
> > EWS was unable to apply the patch. I suggest rebasing and uploading a new one.
> 
> The problem is that it highly depends on bug #12898, so it'd be great if you or someone else could review it.

Sorry, I meant it depends on the patch for the bug #128980.
Comment 5 Javier Fernandez 2014-04-25 06:08:42 PDT
Created attachment 230176 [details]
Patch

Patch rebased.
Comment 6 Javier Fernandez 2014-04-30 14:08:36 PDT
Now that the "grid-template" shorthand has been implemented, perhaps someone could review this patch.
Comment 7 Javier Fernandez 2014-05-01 12:48:02 PDT
Comment on attachment 230176 [details]
Patch

clearing flags because probably this patch is affected by the cause of the regression indentified in bug #132194
Comment 8 Javier Fernandez 2014-05-01 13:28:07 PDT
Created attachment 230607 [details]
Patch

Patch rebased to use the enhanced version of the grid-template shorthand. I've also introduced the changes required to avoid the regression identified in bug #132194
Comment 9 Javier Fernandez 2014-05-06 11:00:34 PDT
ping reviewers.
Comment 10 Benjamin Poulain 2014-05-19 14:22:48 PDT
Comment on attachment 230607 [details]
Patch

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

Quick comments.

> Source/WebCore/css/CSSParser.cpp:4942
> +    RefPtr<CSSValue> autoColumnsValue = nullptr;
> +    RefPtr<CSSValue> autoRowsValue = nullptr;

No need for = nullptr. RefPtr is initialized like that.

> LayoutTests/fast/css-grid-layout/grid-shorthand-get-set.html:36
> +/* Bad values. */
> +
> +#gridWithExplicitAndImplicit {
> +    -webkit-grid: 10px / 20px column;
> +}
> +#gridWithMisplacedNone1 {
> +    -webkit-grid: column 10px / none 20px;
> +}
> +#gridWithMisplacedNone2 {
> +    -webkit-grid: 10px / 20px none;
> +}

That's very little coverage for bad cases.
Comment 11 Javier Fernandez 2014-05-22 02:11:16 PDT
Created attachment 231871 [details]
Patch

Patch rebased and applied suggested changes.
Comment 12 WebKit Commit Bot 2014-05-26 09:25:56 PDT
Comment on attachment 231871 [details]
Patch

Clearing flags on attachment: 231871

Committed r169349: <http://trac.webkit.org/changeset/169349>
Comment 13 WebKit Commit Bot 2014-05-26 09:26:01 PDT
All reviewed patches have been landed.  Closing bug.