[webkit-reviews] review granted: [Bug 68129] change RenderFlexibleBox to act on logical coordinates : [Attachment 108063] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has granted Ojan Vafai <ojan at chromium.org>'s
request for review:
Bug 68129: change RenderFlexibleBox to act on logical coordinates
https://bugs.webkit.org/show_bug.cgi?id=68129

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

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=108063&action=review


r=me

> Source/WebCore/rendering/RenderBlock.h:214
> +    void setLogicalLocationForChild(RenderBox* child, const LayoutPoint&);
>      void setLogicalLeftForChild(RenderBox* child, LayoutUnit logicalLeft,
ApplyLayoutDeltaMode = DoNotApplyLayoutDelta);
>      void setLogicalTopForChild(RenderBox* child, LayoutUnit logicalTop,
ApplyLayoutDeltaMode = DoNotApplyLayoutDelta);

Ultimately you'll probably want to support layout deltas for flexible box
children. You should file a followup bug about this.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:324
> +	   setLogicalLocationForChild(child, IntPoint(logicalLeft,
logicalTop));

You should probably have a placeChild method like the old flexible box did that
handles repainting. Right now you're moving without repainting, which will
result in repaint issues if the child moves without having to get a layout
again.