RenderContainer::positionForCoordinates contains an order of operations error

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Adam Roben (:aroben)]

Here's the code from RenderContainer::positionForCoordinates (also visible at http://trac.webkit.org/projects/webkit/browser/trunk/WebCore/rendering/RenderContainer.cpp#L58 ):

        int top = borderTop() + paddingTop() + isTableRow() ? 0 : renderer->xPos();
        int bottom = top + renderer->contentHeight();
        int left = borderLeft() + paddingLeft() + isTableRow() ? 0 : renderer->yPos();
        int right = left + renderer->contentWidth();

The problem (spotted by prefast) is that the + operator has higher precedence than the ternary operator, so the code evaluates like this (note the parentheses):

        int top = (borderTop() + paddingTop() + isTableRow()) ? 0 : renderer->xPos();
        int bottom = top + renderer->contentHeight();
        int left = (borderLeft() + paddingLeft() + isTableRow()) ? 0 : renderer->yPos();
        int right = left + renderer->contentWidth();

It's easy to fix (just put parentheses around the ternary expression), but I'm not sure how to make a test case.

[Adam Roben (:aroben)]

Sorry, the link to the code should have been http://trac.webkit.org/projects/webkit/browser/trunk/WebCore/rendering/RenderContainer.cpp#L586

(In reply to comment #0)

> It's easy to fix (just put parentheses around the ternary expression)

That "top" goes with "xPos" and "left" goes with "yPos" looks very suspicious as well!

Created attachment 15356 [details]
Test case for RenderContainer::positionForCoordinates

[Adam Roben (:aroben)]

Created attachment 15357 [details]
patch with changelog

[Darin Adler]

Comment on attachment 15357 [details]
patch with changelog

r=me

[Adam Roben (:aroben)]

Landed as r23938