Bug 50183 – Erroneously commented out code in RenderBlock::layoutPositionedObjects()

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Simon Fraser (smfr)]

It looks like this line:

            //if (relayoutChildren && (r->style()->paddingLeft().isPercent() || r->style()->paddingRight().isPercent()))

was left commented by accident.


http://trac.webkit.org/browser/trunk/WebCore/rendering/RenderBlock.cpp?annotate=blame&rev=40461#L1490

[Dave Hyatt]

Created attachment 75061 [details]
Patch

[Dave Hyatt]

Created attachment 75172 [details]
Patch

[Darin Adler]

Comment on attachment 75172 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=75172&action=review

> WebCore/rendering/RenderLayer.cpp:1893
> +    if (!m_vBar && renderer()->hasOverflowClip() && renderer()->style()->overflowY() == OSCROLL)
> +        setHasVerticalScrollbar(true);

Adding this side effect to the width getter seems like a fragile way to structure the code; seems likely that callers will not realize this side effect exists. Is there a clearer way to do this?

[Dave Hyatt]

I could just patch the one call site and hope that's good enough.

[Dave Hyatt]

Created attachment 75176 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 75176 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=75176&action=review

> WebCore/rendering/RenderBlock.cpp:4734
> +            if (!layer()->verticalScrollbar())
> +                layer()->setHasVerticalScrollbar(true);

You could call setHasVerticalScrollbar() unconditionally; it does no work if there's already a vertical scrollbar.

> WebCore/rendering/RenderFlexibleBox.cpp:185
> +        if (!layer()->verticalScrollbar())
> +            layer()->setHasVerticalScrollbar(true);

Ditto.

[Dave Hyatt]

Fixed in r72947.