Bug 60732 – Add support for the grid and inline-grid display types.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Dave Hyatt]

Add support for the grid and inline-grid display types.  For now the RenderGrid object simply acts like a RenderBlock.

[Dave Hyatt]

Created attachment 93466 [details]
Patch

[WebKit Review Bot]

Attachment 93466 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

LayoutTests/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Source/WebCore/css/CSSStyleSelector.cpp:1834:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Total errors found: 2 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Comment on attachment 93466 [details]
Patch

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

> Source/WebCore/GNUmakefile.list.am:2795
>  	Source/WebCore/rendering/RenderFullScreen.h \
> +        Source/WebCore/rendering/RenderGrid.cpp \
> +        Source/WebCore/rendering/RenderGrid.h \

Looks like you might not be matching the whitespace on the previous line here.

> Source/WebCore/css/CSSStyleSelector.cpp:1838
> +            || (e && e->document()->documentElement() == e))

Would be nice to add a helper for e->document()->documentElement() == e

> Source/WebCore/rendering/RenderGrid.cpp:33
> +    :RenderBlock(node)

Space after the :

> Source/WebCore/rendering/RenderGrid.h:38
> +    virtual const char* renderName() const;

Can't this be private?

> LayoutTests/fast/grid/empty-grids.html:12
> +<p>In this paragraph <span class="grid" style="float:left"></span> there should be <span class="grid" style="float:right"></span>
> +two empty grids.  One will float to the left and one will float to the right.  They should both be empty.</p>

If the text were in HTML comments, you'd end up with a cross-platform pixel result.

[Early Warning System Bot]

Comment on attachment 93466 [details]
Patch

Attachment 93466 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/8698093

[WebKit Review Bot]

Comment on attachment 93466 [details]
Patch

Attachment 93466 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8698113

New failing tests:
tables/mozilla/bugs/bug32447.html
tables/mozilla/bugs/bug159108.html
tables/mozilla/bugs/bug120107.html
tables/mozilla/bugs/bug40828.html
tables/mozilla/bugs/bug25663.html
tables/mozilla/bugs/bug145572.html
tables/mozilla/bugs/bug34538.html
tables/mozilla/bugs/bug106158-2.html
tables/mozilla/bugs/bug23299.html
tables/mozilla/bugs/bug45055-2.html
tables/mozilla/bugs/bug24661.html
tables/mozilla/bugs/bug16252.html
tables/mozilla/bugs/bug4093.html
tables/mozilla/bugs/bug1261.html
tables/mozilla/bugs/bug138725.html
tables/mozilla/bugs/bug12910.html
tables/mozilla/bugs/bug13196.html
tables/mozilla/bugs/bug120364.html
tables/mozilla/bugs/bug2585.html
tables/mozilla/bugs/bug106158-1.html

[WebKit Review Bot]

Created attachment 93482 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Darin Adler]

Comment on attachment 93466 [details]
Patch

Clearing review flag from this obsolete patch.

[Julien Chaffraix]

Created attachment 129378 [details]
Rebaselined change, integrated Simon's comment, turned the test cases to ref-tests.

[Tony Chang]

Comment on attachment 129378 [details]
Rebaselined change, integrated Simon's comment, turned the test cases to ref-tests.

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

> Source/WebCore/GNUmakefile.list.am:3550
> +        Source/WebCore/rendering/RenderGrid.cpp \
> +        Source/WebCore/rendering/RenderGrid.h \

Nit: I thought make required tabs, not spaces.  Anyway, the other lines use tabs.

[Julien Chaffraix]

Created attachment 148691 [details]
Rebaselined once more, let's try to land it.

[Build Bot]

Comment on attachment 148691 [details]
Rebaselined once more, let's try to land it.

Attachment 148691 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13018046

[Early Warning System Bot]

Comment on attachment 148691 [details]
Rebaselined once more, let's try to land it.

Attachment 148691 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13017059

[Build Bot]

Comment on attachment 148691 [details]
Rebaselined once more, let's try to land it.

Attachment 148691 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13018054

[Early Warning System Bot]

Comment on attachment 148691 [details]
Rebaselined once more, let's try to land it.

Attachment 148691 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13011065

[WebKit Review Bot]

Comment on attachment 148691 [details]
Rebaselined once more, let's try to land it.

Attachment 148691 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13022028

[Julien Chaffraix]

Created attachment 148714 [details]
Grumble, grumble. ENABLE_CSS_GRID_LAYOUT is dead...

[Early Warning System Bot]

Comment on attachment 148714 [details]
Grumble, grumble. ENABLE_CSS_GRID_LAYOUT is dead...

Attachment 148714 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13012111

[Early Warning System Bot]

Comment on attachment 148714 [details]
Grumble, grumble. ENABLE_CSS_GRID_LAYOUT is dead...

Attachment 148714 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13027001

[Tony Chang]

Comment on attachment 148714 [details]
Grumble, grumble. ENABLE_CSS_GRID_LAYOUT is dead...

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

Please make sure the Qt ews bots are green before landing.

> Source/WebCore/ChangeLog:21
> +        * CMakeLists.txt:
> +        * GNUmakefile.list.am:
> +        * WebCore.gypi:
> +        * WebCore.vcproj/WebCore.vcproj:
> +        * WebCore.xcodeproj/project.pbxproj:

Qt failure is because Target.pri is missing.

> Source/WebCore/rendering/RenderObject.cpp:157
>      case GRID:
>      case INLINE_GRID:
> +        return new (arena) RenderGrid(node);

Nit: I would move this below FLEX and INLINE_FLEX to match the order of the enum.

[Julien Chaffraix]

Created attachment 148824 [details]
Patch for EWS testing / landing. Thanks Tony.

[Julien Chaffraix]

Created attachment 148871 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 148871 [details]
Patch for landing

Clearing flags on attachment: 148871

Committed r120984: <http://trac.webkit.org/changeset/120984>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.