Bug 60732 - Add support for the grid and inline-grid display types.
Summary: Add support for the grid and inline-grid display types.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks: 60731
  Show dependency treegraph
 
Reported: 2011-05-12 15:16 PDT by Dave Hyatt
Modified: 2012-06-21 17:36 PDT (History)
13 users (show)

See Also:


Attachments
Patch (66.54 KB, patch)
2011-05-13 09:35 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-02 (257.69 KB, application/zip)
2011-05-13 10:46 PDT, WebKit Review Bot
no flags Details
Rebaselined change, integrated Simon's comment, turned the test cases to ref-tests. (20.08 KB, patch)
2012-02-28 19:58 PST, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Rebaselined once more, let's try to land it. (20.33 KB, patch)
2012-06-20 17:34 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Grumble, grumble. ENABLE_CSS_GRID_LAYOUT is dead... (20.19 KB, patch)
2012-06-20 19:41 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Patch for EWS testing / landing. Thanks Tony. (21.20 KB, patch)
2012-06-21 10:00 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Patch for landing (21.25 KB, patch)
2012-06-21 13:28 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 2011-05-12 15:16:01 PDT
Add support for the grid and inline-grid display types.  For now the RenderGrid object simply acts like a RenderBlock.
Comment 1 Dave Hyatt 2011-05-13 09:35:50 PDT
Created attachment 93466 [details]
Patch
Comment 2 WebKit Review Bot 2011-05-13 09:39:25 PDT
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.
Comment 3 Simon Fraser (smfr) 2011-05-13 09:40:43 PDT
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.
Comment 4 Early Warning System Bot 2011-05-13 10:03:33 PDT
Comment on attachment 93466 [details]
Patch

Attachment 93466 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/8698093
Comment 5 WebKit Review Bot 2011-05-13 10:46:15 PDT
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
Comment 6 WebKit Review Bot 2011-05-13 10:46:18 PDT
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
Comment 7 Darin Adler 2011-06-18 12:34:38 PDT
Comment on attachment 93466 [details]
Patch

Clearing review flag from this obsolete patch.
Comment 8 Julien Chaffraix 2012-02-28 19:58:58 PST
Created attachment 129378 [details]
Rebaselined change, integrated Simon's comment, turned the test cases to ref-tests.
Comment 9 Tony Chang 2012-02-29 10:40:35 PST
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.
Comment 10 Julien Chaffraix 2012-06-20 17:34:16 PDT
Created attachment 148691 [details]
Rebaselined once more, let's try to land it.
Comment 11 Build Bot 2012-06-20 18:02:46 PDT
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
Comment 12 Early Warning System Bot 2012-06-20 18:08:36 PDT
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
Comment 13 Build Bot 2012-06-20 18:09:43 PDT
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
Comment 14 Early Warning System Bot 2012-06-20 18:26:50 PDT
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
Comment 15 WebKit Review Bot 2012-06-20 18:46:57 PDT
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
Comment 16 Julien Chaffraix 2012-06-20 19:41:32 PDT
Created attachment 148714 [details]
Grumble, grumble. ENABLE_CSS_GRID_LAYOUT is dead...
Comment 17 Early Warning System Bot 2012-06-20 19:57:07 PDT
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
Comment 18 Early Warning System Bot 2012-06-20 20:33:16 PDT
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
Comment 19 Tony Chang 2012-06-21 09:54:16 PDT
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.
Comment 20 Julien Chaffraix 2012-06-21 10:00:08 PDT
Created attachment 148824 [details]
Patch for EWS testing / landing. Thanks Tony.
Comment 21 Julien Chaffraix 2012-06-21 13:28:40 PDT
Created attachment 148871 [details]
Patch for landing
Comment 22 WebKit Review Bot 2012-06-21 17:36:24 PDT
Comment on attachment 148871 [details]
Patch for landing

Clearing flags on attachment: 148871

Committed r120984: <http://trac.webkit.org/changeset/120984>
Comment 23 WebKit Review Bot 2012-06-21 17:36:31 PDT
All reviewed patches have been landed.  Closing bug.