[webkit-reviews] review granted: [Bug 60732] Add support for the grid and inline-grid display types. : [Attachment 93466] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Dave Hyatt
<hyatt at apple.com>'s request for review:
Bug 60732: Add support for the grid and inline-grid display types.
https://bugs.webkit.org/show_bug.cgi?id=60732

Attachment 93466: Patch
https://bugs.webkit.org/attachment.cgi?id=93466&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
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.