[webkit-reviews] review denied: [Bug 100399] [CSS Exclusions] shape-outside on floats for rectangle shapes positioning : [Attachment 183572] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Bem Jones-Bey <bjonesbe at adobe.com>'s
request for review:
Bug 100399: [CSS Exclusions] shape-outside on floats for rectangle shapes
positioning
https://bugs.webkit.org/show_bug.cgi?id=100399

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

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


r-

> Source/WebCore/rendering/RenderBlock.cpp:3115
> -LayoutPoint RenderBlock::flipFloatForWritingModeForChild(const
FloatingObject* child, const LayoutPoint& point) const
> +LayoutPoint RenderBlock::flipFloatForWritingModeForChild(const
FloatingObject* child, const LayoutPoint& point, bool paintTime) const

The "ForChild" here is actually meant to imply the 2 * multiplier. It is
analogous to RenderBox flipForWritingModeForChild. I don't think we want to
introduce an inscrutable boolean here whose meaning cannot be discerned when
looking at call sites.

If you want to be consistent with the RenderBox methods, I would just make a
second method called flipFloatForWritingMode that doesn't do the double
subtraction.

I'd also be fine with an enum instead of a boolean so that the purpose of the
value at call sites is clear. Either approach works for me.