[webkit-reviews] review denied: [Bug 93547] [CSS Exclusions] Enable shape-inside for percentage lengths based on logical height : [Attachment 162899] Incorporating feedback

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Bear Travis <betravis at adobe.com>'s
request for review:
Bug 93547: [CSS Exclusions] Enable shape-inside for percentage lengths based on
logical height
https://bugs.webkit.org/show_bug.cgi?id=93547

Attachment 162899: Incorporating feedback
https://bugs.webkit.org/attachment.cgi?id=162899&action=review

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


Comments below.

> Source/WebCore/rendering/RenderBlock.cpp:1397
> -void RenderBlock::computeInitialRegionRangeForBlock()
> +void RenderBlock::computeInitialLogicalSizeBasedMeasurements()

My suggestion here would be to retain computeInitialRegionRangeForBlock and
have it be called by computeInitialLogicalSizeBasedMeasurements. Then have your
own additional function that computeInitialLogicalSizeBasedMeasurements calls
to handle exclusions.

Note the term "size" is wrong here, since all you care about is logical height,
so you should rename to computeLogicalHeightBasedMeasurements. I think you can
drop the term "initial" since that really only applied to region ranges and was
implying that the region range might be corrected later on.

> Source/WebCore/rendering/RenderBox.h:40
> +enum PositionType { InFlowPosition, OutOfFlowPosition };

I'm not following why this needs to be added. I don't like it.