Bug 119801 - [CSS Grid Layout] Fix grid position resolution
Summary: [CSS Grid Layout] Fix grid position resolution
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sergio Villar Senin
URL:
Keywords: BlinkMergeCandidate
Depends on: 119552
Blocks: 60731 119756
  Show dependency treegraph
 
Reported: 2013-08-14 09:01 PDT by Sergio Villar Senin
Modified: 2013-08-28 03:25 PDT (History)
10 users (show)

See Also:


Attachments
Patch (15.14 KB, patch)
2013-08-22 04:12 PDT, Sergio Villar Senin
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sergio Villar Senin 2013-08-14 09:01:22 PDT
Neither grid-{column|row}-end nor negative values are properly handled in our resolution code right now.

In order to fix it we should merge these 3 changes:

---
    r148833
    Fix the grid-{end|after} position resolution
    
    Our resolution code was resolving both positions in the same way
    but that doesn't work: in grid-rows: 2 / 3, the 2 lines, even if
    they are different numbers, refers to the same grid position so
    using the same formula to resolve them cannot work.

---
    r148878
   Refactor grid size estimation to use resolveGridPositionsFromStyle
    
    resolveGridPositionsFromStyle required to have the grid filled so that
    we resolve negative indexes against the grid's final edge. The
    specification (and the code) changed to use the 'explicit grid' which
    is independent of the grid items and depends only on the style.

---
    r150403
    This change just fixes resolveGridPositionFromStyle to handle the
    negative grid position properly. While at it, factored the position
    adjustment logic into adjustGridPostionForSide.
Comment 1 Sergio Villar Senin 2013-08-22 04:12:56 PDT
Created attachment 209342 [details]
Patch
Comment 2 Sergio Villar Senin 2013-08-22 04:15:08 PDT
The final patch is not a direct merge of the three patches from Blink. I removed some stuff (like tests with span and negative indexes) that will be included in future patches (like in the one for 119756).
Comment 3 Andreas Kling 2013-08-27 07:53:54 PDT
Comment on attachment 209342 [details]
Patch

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

> Source/WebCore/ChangeLog:16
> +        resolveGridPositionsFormStyle() so we can use it for the grid size

Typo, resolveGridPositions_From_Style.

> Source/WebCore/rendering/RenderGrid.cpp:768
> +        ASSERT(position.integerPosition());

This is a well-placed assert. I was worried about negative values here.
Comment 4 Sergio Villar Senin 2013-08-28 03:25:51 PDT
Committed r154731: <http://trac.webkit.org/changeset/154731>