[webkit-reviews] review denied: [Bug 96704] Simplify some code in RenderBox::computePercentageLogicalHeight : [Attachment 163996] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Ojan Vafai <ojan at chromium.org>'s
request for review:
Bug 96704: Simplify some code in RenderBox::computePercentageLogicalHeight
https://bugs.webkit.org/show_bug.cgi?id=96704

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

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=163996&action=review


> Source/WebCore/rendering/RenderBox.cpp:-2129
> -    // In quirks mode, blocks with auto height are skipped, and we keep
looking for an enclosing
> -    // block that may have a specified height and then use it. In strict
mode, this violates the
> -    // specification, which states that percentage heights just revert to
auto if the containing
> -    // block has an auto height. We still skip anonymous containing blocks
in both modes, though, and look

I kind of like the comment about how quirks mode and standards mode are
different.  Maybe just rewrite it into a shorter sentence?

> Source/WebCore/rendering/RenderBox.cpp:2130
> +    while ((document()->inQuirksMode() || cb->isAnonymousBlock())
> +	   && !cb->isRenderView() && !cb->isTableCell() &&
!cb->isOutOfFlowPositioned() && cb->style()->logicalHeight().isAuto() &&
isHorizontalWritingMode() == cb->isHorizontalWritingMode()) {

I find this impossible to read, even the old code was easier to read.  Can we
make a helper function or something?