[webkit-changes] [14351] trunk/WebCore

[Allan Sandfeld Jensen]

On Saturday 13 May 2006 16:58, ap at opensource.apple.com wrote:
> 2006-05-13  Sam Weinig  <sam.weinig at gmail.com>
>
>         Reviewed by Hyatt, landed by ap.
>
> WebCore:
>         Patch for http://bugzilla.opendarwin.org/show_bug.cgi?id=7604
>         calcAbsoluteHorizontalValues() is being getting passed arguments
>         in the wrong order in calcAbsoluteHorizontal()
>
>         Cleans up the RenderBox code for absolutely positioned elements
>         and adds new functions for replaced absolutely positioned
>         elements. Now uses Length so that magic number -666666 for
>         auto lengths is no longer used.
>
>         * rendering/RenderBox.cpp:
>         (WebCore::RenderBox::calcAbsoluteHorizontal):
>         (WebCore::RenderBox::calcAbsoluteHorizontalValues):
>         (WebCore::RenderBox::calcAbsoluteVertical):
>         (WebCore::RenderBox::calcAbsoluteVerticalValues):
>         (WebCore::RenderBox::calcAbsoluteHorizontalReplaced): Handle
> replaced case separately.
>         (WebCore::RenderBox::calcAbsoluteVerticalReplaced): ditto.
>         * rendering/RenderBox.h:
>

I just merged this patch to KHTML, and noticed a regression:

In the overconstrained case in calcAbsoluteVerticalReplaced() (step 6) you 
discard information about top, margin-top and margin-bottom.

It should look like this: 
    /*-----------------------------------------------------------------------*\
     * 6. If at this point the values are over-constrained, ignore the value
     *    for 'bottom' and solve for that value.    
    \*-----------------------------------------------------------------------*/
    else {
        m_marginTop = marginTop.width(containerHeight);
        m_marginBottom = marginBottom.width(containerHeight);
        topValue = top.width(containerHeight);

        // Solve for 'bottom'
        // NOTE: It is not necessary to solve for 'bottom' because we don't ever
        // use the value.
    }


There is ofcourse the same problem in calcAbsoluteVerticalReplaced() where it 
should look like:
    /*-----------------------------------------------------------------------*\
     * 6. If at this point the values are over-constrained, ignore the value
     *    for either 'left' (in case the 'direction' property of the
     *    containing block is 'rtl') or 'right' (in case 'direction' is
     *    'ltr') and solve for that value.
    \*-----------------------------------------------------------------------*/
    else  {
        m_marginLeft = marginLeft.width(containerWidth);
        m_marginRight = marginRight.width(containerWidth);
        if (containerDirection  == LTR) {
            leftValue = left.width(containerWidth);
            rightValue = availableSpace - (leftValue + m_marginLeft + m_marginRight);
        } 
        else {
            rightValue = right.width(containerWidth);
            leftValue = availableSpace - (rightValue + m_marginLeft + m_marginRight);
        }
    }



Regards
`Allan