Bug 96704 – Simplify some code in RenderBox::computePercentageLogicalHeight

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ojan Vafai]

Simplify some code in RenderBox::computePercentageLogicalHeight

[Ojan Vafai]

Created attachment 163996 [details]
Patch

[Tony Chang]

Comment on attachment 163996 [details]
Patch

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?

[Tony Chang]

(In reply to comment #2)
> (From update of attachment 163996 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163996&action=review
> 
> > 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?

Something like,
bool shouldTraverseParentForPercentHeight() {
    return document()->inQuirksMode() || cb->isAnonymousBlock();
}

and

bool useParentHeightForPrecentHeight() {
    return !cb->isTableCell() && !cb->isOutOfFlowPositioned() && cb->style()->logicalHeight().isAuto() && isHorizontalWritingMode() == cb->isHorizontalWritingMode();
}

So the while would be:

while (!cb->isRenderView() && shouldTraverseParentForPercentHeight() && useParentHeightForPercentHeight()) {
    ...
}

[Ojan Vafai]

Created attachment 164195 [details]
Patch

[Ojan Vafai]

Committed r128633: <http://trac.webkit.org/changeset/128633>