Bug 105548 - [CSS Grid Layout] Add the plumbing to parse -webkit-grid-{column,row}-span
Summary: [CSS Grid Layout] Add the plumbing to parse -webkit-grid-{column,row}-span
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 60731 103314
  Show dependency treegraph
 
Reported: 2012-12-20 09:59 PST by Xan Lopez
Modified: 2013-02-22 13:12 PST (History)
11 users (show)

See Also:


Attachments
column-row-span.diff (20.87 KB, patch)
2012-12-21 04:52 PST, Xan Lopez
jchaffraix: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Xan Lopez 2012-12-20 09:59:37 PST
First step towards fixing bug #103314, I'll attach the first patch shortly.
Comment 1 Xan Lopez 2012-12-21 04:52:15 PST
Created attachment 180500 [details]
column-row-span.diff

As done in previous features, this only adds the parsing bits, no effects on layout yet.

The two things I consider are probably up for debate:

- I use GridPosition to store the span, it might be better to create a new type (GridSpan?)
- I actually store the span in the item data. Perhaps it would be better to store a second GridPosition (m_gridColumnEnd, m_gridRowEnd?), and figure out the span doing end-start.
Comment 2 Julien Chaffraix 2013-01-10 16:11:23 PST
Comment on attachment 180500 [details]
column-row-span.diff

> The two things I consider are probably up for debate:
> - I use GridPosition to store the span, it might be better to create a new type (GridSpan?)
> - I actually store the span in the item data. Perhaps it would be better to store a second GridPosition (m_gridColumnEnd, m_gridRowEnd?), and figure out the span doing end-start.

I don't think it makes sense to have GridPosition embeds the span information: it will be difficult not to store both the span and the end GridPosition as dynamic updates could easily add / remove or update one or the other - including changing the order in which they occur. Also the end GridPosition would override the span if present which adds some complexity and could make the transition tricky if we try to save on storing the spans.

Also, we should be careful not to move the parsing too much forward with respect to implementing the layout (if only to keep some feature detection for the early adopters). The current grammar is correlated with a subset of the layout that is powerful enough (auto sizing, minmax), yet whose complexity is manageable. Adding support for spanning columns / rows adds a lot of complexity which I would rather delay until minmax implementation is stable enough.
Comment 3 Julien Chaffraix 2013-01-30 09:04:10 PST
Comment on attachment 180500 [details]
column-row-span.diff

We are up to a point where I would be fine to see this patch land (minmax layout is working). Xan, do you feel like adding a version that keeps the span as an int. We would keep the extra GridPosition to handle grid-definition-{columns|rows}.
Comment 4 Julien Chaffraix 2013-01-31 11:11:18 PST
Comment on attachment 180500 [details]
column-row-span.diff

The specification has changed and they removed grid-{column|row}-span. Talking with Tab, it seems we should wait a bit before diving into this as it could change next week at the W3C face-to-face (I should have checked the facts first before saying we are ready).

r-'ing the patch for now, we can always revive it if the situation changes.
Comment 5 Julien Chaffraix 2013-02-22 13:12:44 PST
The specification has completely changed and the concept of grid-{column,row}-span was removed, inlined into grid-{start|end} / grid-{before|after}. Example:

grid-start: 3; grid-end: span 2;

(Beware that now integers corresponds to the matching edge, ie 3 from the 'start' and spans to 2 from the 'end')

I don't think we can salvage anything from this patch. Feel free to reopen if I missed something.