[webkit-reviews] review granted: [Bug 76664] Incorrect positioning of floating pseudo-elements in table captions : [Attachment 123599] Patch

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has granted Robert Hogan
<robert at webkit.org>'s request for review:
Bug 76664: Incorrect positioning of floating pseudo-elements in table captions
https://bugs.webkit.org/show_bug.cgi?id=76664

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

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=123599&action=review


> Source/WebCore/rendering/RenderTable.cpp:277
> +	   caption->layoutIfNeeded();

It should be caption->layout(); directly as you know you need layout in this
branch.

> Source/WebCore/rendering/RenderTable.h:251
> +    void layoutCaptionIfNeeded(RenderTableCaption*);

layoutCaptionIfNeeded is definitely a misnomer here (look at how the
*IfNeeded() functions are implemented and you deviate from the pattern).

I really prefer layoutCaption to name the function but here are some
alternatives if you want to avoid the word 'layout': placeCaption(),
correctCaptionPosition() or adjustCaptionPosition()

> LayoutTests/fast/table/caption-encloses-overhanging-float.html:10
> +    <table>

Nit: add a comment with this bug URL, possibly a description of the expected
output (like 'you should see no red')