Changeset 93616 in webkit


Ignore:
Timestamp:
Aug 23, 2011 11:11:44 AM (13 years ago)
Author:
robert@webkit.org
Message:

div align="center" rendering problem
​https://bugs.webkit.org/show_bug.cgi?id=4860

Reviewed by David Hyatt.

Source/WebCore:

Tests: fast/inline/absolute-positioned-block-in-centred-block.html

fast/inline/absolute-positioned-inline-in-centred-block.html

When an element with absolute position was the sole or first child of a render block with
centred alignment, it wasn't obeying its parent's alignment. However it would obey the
alignment if it was preceded by some text. The problem was that the element's render object
was getting skipped as leading white space, so it was not included in a normal line block in
a bidi run. Instead, its position was getting set by RenderBlockLineLayout::setStaticPositions()
which does not pay attention to alignment. Preceding the element with some text allowed the object
to get included in a Bidi run and so get a linebox which would get properly aligned.

The fix is to get RenderBlockLineLayout::setStaticPositions() to obey the alignment specified by
the object's container. This allows WebKit to get the same result on the test as Firefox and IE.
Opera has the same bug as unpatched WebKit.

  • rendering/RenderBlock.cpp:

(WebCore::RenderBlock::adjustPositionedBlock): use startAlignedOffsetForBlock

  • rendering/RenderBlock.h:
  • rendering/RenderBlockLineLayout.cpp:

(WebCore::RenderBlock::updateLogicalWidthForAlignment):
(WebCore::RenderBlock::computeInlineDirectionPositionsForLine): Move the alignment check to updateLogicalWidthForAlignment.
(WebCore::setStaticPositions): use startAlignedOffsetForLine and use startAlignedOffsetForBlock
(WebCore::RenderBlock::LineBreaker::nextLineBreak): use startAlignedOffsetForLine
(WebCore::RenderBlock::startAlignedOffsetForBlock): New function, find the aligned offset using updateLogicalWidthForAlignment
(WebCore::RenderBlock::startAlignedOffsetForLine): ditto

  • rendering/RenderBlock.h:
  • rendering/RenderBlockLineLayout.cpp:

(WebCore::RenderBlock::updateLogicalWidthForAlignment):
(WebCore::RenderBlock::computeInlineDirectionPositionsForLine): Move the alignment check to updateLogicalWidthForAlignment.
(WebCore::setStaticPositions): use startAlignedOffsetForLine.
(WebCore::RenderBlock::startAlignedOffsetForLine): New function, find the aligned offset using updateLogicalWidthForAlignment

LayoutTests:

  • fast/inline/absolute-positioned-block-in-centred-block-expected.png: Added.
  • fast/inline/absolute-positioned-block-in-centred-block-expected.txt: Added.
  • fast/inline/absolute-positioned-block-in-centred-block.html: Added.
  • fast/inline/absolute-positioned-inline-in-centred-block-expected.png: Added.
  • fast/inline/absolute-positioned-inline-in-centred-block-expected.txt: Added.
  • fast/inline/absolute-positioned-inline-in-centred-block.html: Added.
  • platform/chromium-linux/fast/repaint/block-layout-inline-children-float-positioned-expected.png:
  • platform/chromium-linux/fast/repaint/block-layout-inline-children-float-positioned-expected.txt: Both of these two tests were based on the incorrect behaviour of ignoring the alignment specified by the container of an element absolute position when there was no text preceding the element. The updated results agree with Firefox and IE.
Location:
trunk
Files:
6 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r93614 r93616  
     12011-07-25  Robert Hogan  <robert@webkit.org>
     2
     3        div align="center" rendering problem
     4        https://bugs.webkit.org/show_bug.cgi?id=4860
     5
     6        Reviewed by David Hyatt.
     7
     8        * fast/inline/absolute-positioned-block-in-centred-block-expected.png: Added.
     9        * fast/inline/absolute-positioned-block-in-centred-block-expected.txt: Added.
     10        * fast/inline/absolute-positioned-block-in-centred-block.html: Added.
     11        * fast/inline/absolute-positioned-inline-in-centred-block-expected.png: Added.
     12        * fast/inline/absolute-positioned-inline-in-centred-block-expected.txt: Added.
     13        * fast/inline/absolute-positioned-inline-in-centred-block.html: Added.
     14        * platform/chromium-linux/fast/repaint/block-layout-inline-children-float-positioned-expected.png:
     15        * platform/chromium-linux/fast/repaint/block-layout-inline-children-float-positioned-expected.txt:
     16          Both of these two tests were based on the incorrect behaviour of ignoring the alignment specified
     17          by the container of an element absolute position when there was no text preceding the element.
     18          The updated results agree with Firefox and IE.
     19
    1202011-08-23  Julien Chaffraix  <jchaffraix@webkit.org>
    221

  • trunk/LayoutTests/platform/chromium-linux/fast/repaint/block-layout-inline-children-float-positioned-expected.txt

    r68121 r93616  
    1515            RenderText {#text} at (0,0) size 277x19
    1616              text run at (0,0) width 277: "the quick brown fox jumped over the lazy dog"
    17 layer at (1,37) size 277x20
    18   RenderBlock (positioned) {SPAN} at (1,37) size 277x20
     17layer at (162,37) size 277x20
     18  RenderBlock (positioned) {SPAN} at (162,37) size 277x20
    1919    RenderText {#text} at (0,0) size 277x19
    2020      text run at (0,0) width 277: "the quick brown fox jumped over the lazy dog"

  • trunk/Source/WebCore/ChangeLog

    r93615 r93616  
     12011-07-25  Robert Hogan  <robert@webkit.org>
     2
     3        div align="center" rendering problem
     4        https://bugs.webkit.org/show_bug.cgi?id=4860
     5
     6        Reviewed by David Hyatt.
     7
     8        Tests: fast/inline/absolute-positioned-block-in-centred-block.html
     9               fast/inline/absolute-positioned-inline-in-centred-block.html
     10
     11        When an element with absolute position was the sole or first child of a render block with
     12        centred alignment, it wasn't obeying its parent's alignment. However it would obey the
     13        alignment if it was preceded by some text. The problem was that the element's render object
     14        was getting skipped as leading white space, so it was not included in a normal line block in
     15        a bidi run. Instead, its position was getting set by RenderBlockLineLayout::setStaticPositions()
     16        which does not pay attention to alignment. Preceding the element with some text allowed the object
     17        to get included in a Bidi run and so get a linebox which would get properly aligned.
     18
     19        The fix is to get RenderBlockLineLayout::setStaticPositions() to obey the alignment specified by
     20        the object's container. This allows WebKit to get the same result on the test as Firefox and IE.
     21        Opera has the same bug as unpatched WebKit.
     22
     23        * rendering/RenderBlock.cpp:
     24        (WebCore::RenderBlock::adjustPositionedBlock): use startAlignedOffsetForBlock
     25        * rendering/RenderBlock.h:
     26        * rendering/RenderBlockLineLayout.cpp:
     27        (WebCore::RenderBlock::updateLogicalWidthForAlignment):
     28        (WebCore::RenderBlock::computeInlineDirectionPositionsForLine): Move the alignment check to updateLogicalWidthForAlignment.
     29        (WebCore::setStaticPositions): use startAlignedOffsetForLine and use startAlignedOffsetForBlock
     30        (WebCore::RenderBlock::LineBreaker::nextLineBreak): use startAlignedOffsetForLine
     31        (WebCore::RenderBlock::startAlignedOffsetForBlock): New function, find the aligned offset using updateLogicalWidthForAlignment
     32        (WebCore::RenderBlock::startAlignedOffsetForLine): ditto
     33
     34        * rendering/RenderBlock.h:
     35        * rendering/RenderBlockLineLayout.cpp:
     36        (WebCore::RenderBlock::updateLogicalWidthForAlignment):
     37        (WebCore::RenderBlock::computeInlineDirectionPositionsForLine): Move the alignment check to updateLogicalWidthForAlignment.
     38        (WebCore::setStaticPositions): use startAlignedOffsetForLine.
     39        (WebCore::RenderBlock::startAlignedOffsetForLine): New function, find the aligned offset using updateLogicalWidthForAlignment
     40
    1412011-08-23  Adrienne Walker  <enne@google.com>
    242

  • trunk/Source/WebCore/rendering/RenderBlock.cpp

    r93556 r93616  
    14761476    RenderLayer* childLayer = child->layer();
    14771477       
    1478     childLayer->setStaticInlinePosition(borderAndPaddingStart());
     1478    childLayer->setStaticInlinePosition(startAlignedOffsetForBlock(logicalHeight(), false));
    14791479
    14801480    LayoutUnit logicalTop = logicalHeight();

  • trunk/Source/WebCore/rendering/RenderBlock.h

    r93556 r93616  
    130130    LayoutUnit logicalLeftOffsetForLine(LayoutUnit position, bool firstLine) const { return logicalLeftOffsetForLine(position, logicalLeftOffsetForContent(), firstLine); }
    131131    LayoutUnit startOffsetForLine(LayoutUnit position, bool firstLine) const { return style()->isLeftToRightDirection() ? logicalLeftOffsetForLine(position, firstLine) : logicalRightOffsetForLine(position, firstLine); }
     132    LayoutUnit startAlignedOffsetForLine(LayoutUnit position, bool firstLine);
     133    LayoutUnit startAlignedOffsetForBlock(LayoutUnit position, bool firstLine);
    132134
    133135    virtual VisiblePosition positionForPoint(const LayoutPoint&);
    … …  
    223225    LayoutUnit collapsedMarginAfterForChild(RenderBox* child) const;
    224226
     227    void updateLogicalWidthForAlignment(const ETextAlign&, BidiRun* trailingSpaceRun, float& logicalLeft, float& totalLogicalWidth, float& availableLogicalWidth, int expansionOpportunityCount);
     228
    225229    virtual void updateFirstLetter();
    226230

  • trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp

    r93559 r93616  
    550550}
    551551
     552void RenderBlock::updateLogicalWidthForAlignment(const ETextAlign& textAlign, BidiRun* trailingSpaceRun, float& logicalLeft, float& totalLogicalWidth, float& availableLogicalWidth, int expansionOpportunityCount)
     553{
     554    // Armed with the total width of the line (without justification),
     555    // we now examine our text-align property in order to determine where to position the
     556    // objects horizontally. The total width of the line can be increased if we end up
     557    // justifying text.
     558    switch (textAlign) {
     559    case LEFT:
     560    case WEBKIT_LEFT:
     561        updateLogicalWidthForLeftAlignedBlock(style()->isLeftToRightDirection(), trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth);
     562        break;
     563    case JUSTIFY:
     564        adjustInlineDirectionLineBounds(expansionOpportunityCount, logicalLeft, availableLogicalWidth);
     565        if (expansionOpportunityCount) {
     566            if (trailingSpaceRun) {
     567                totalLogicalWidth -= trailingSpaceRun->m_box->logicalWidth();
     568                trailingSpaceRun->m_box->setLogicalWidth(0);
     569            }
     570            break;
     571        }
     572        // fall through
     573    case TAAUTO:
     574        // for right to left fall through to right aligned
     575        if (style()->isLeftToRightDirection()) {
     576            if (totalLogicalWidth > availableLogicalWidth && trailingSpaceRun)
     577                trailingSpaceRun->m_box->setLogicalWidth(max<float>(0, trailingSpaceRun->m_box->logicalWidth() - totalLogicalWidth + availableLogicalWidth));
     578            break;
     579        }
     580    case RIGHT:
     581    case WEBKIT_RIGHT:
     582        updateLogicalWidthForRightAlignedBlock(style()->isLeftToRightDirection(), trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth);
     583        break;
     584    case CENTER:
     585    case WEBKIT_CENTER:
     586        updateLogicalWidthForCenterAlignedBlock(style()->isLeftToRightDirection(), trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth);
     587        break;
     588    case TASTART:
     589        if (style()->isLeftToRightDirection())
     590            updateLogicalWidthForLeftAlignedBlock(style()->isLeftToRightDirection(), trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth);
     591        else
     592            updateLogicalWidthForRightAlignedBlock(style()->isLeftToRightDirection(), trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth);
     593        break;
     594    case TAEND:
     595        if (style()->isLeftToRightDirection())
     596            updateLogicalWidthForRightAlignedBlock(style()->isLeftToRightDirection(), trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth);
     597        else
     598            updateLogicalWidthForLeftAlignedBlock(style()->isLeftToRightDirection(), trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth);
     599        break;
     600    }
     601}
     602
    552603void RenderBlock::computeInlineDirectionPositionsForLine(RootInlineBox* lineBox, const LineInfo& lineInfo, BidiRun* firstRun, BidiRun* trailingSpaceRun, bool reachedEnd,
    553604                                                         GlyphOverflowAndFallbackFontsMap& textBoxDataMap, VerticalPositionCache& verticalPositionCache)
    … …  
    606657    }
    607658
    608     // Armed with the total width of the line (without justification),
    609     // we now examine our text-align property in order to determine where to position the
    610     // objects horizontally.  The total width of the line can be increased if we end up
    611     // justifying text.
    612     switch (textAlign) {
    613         case LEFT:
    614         case WEBKIT_LEFT:
    615             updateLogicalWidthForLeftAlignedBlock(style()->isLeftToRightDirection(), trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth);
    616             break;
    617         case JUSTIFY:
    618             adjustInlineDirectionLineBounds(expansionOpportunityCount, logicalLeft, availableLogicalWidth);
    619             if (expansionOpportunityCount) {
    620                 if (trailingSpaceRun) {
    621                     totalLogicalWidth -= trailingSpaceRun->m_box->logicalWidth();
    622                     trailingSpaceRun->m_box->setLogicalWidth(0);
    623                 }
    624                 break;
    625             }
    626             // fall through
    627         case TAAUTO:
    628             // for right to left fall through to right aligned
    629             if (style()->isLeftToRightDirection()) {
    630                 if (totalLogicalWidth > availableLogicalWidth && trailingSpaceRun)
    631                     trailingSpaceRun->m_box->setLogicalWidth(max<float>(0, trailingSpaceRun->m_box->logicalWidth() - totalLogicalWidth + availableLogicalWidth));
    632                 break;
    633             }
    634         case RIGHT:
    635         case WEBKIT_RIGHT:
    636             updateLogicalWidthForRightAlignedBlock(style()->isLeftToRightDirection(), trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth);
    637             break;
    638         case CENTER:
    639         case WEBKIT_CENTER:
    640             updateLogicalWidthForCenterAlignedBlock(style()->isLeftToRightDirection(), trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth);
    641             break;
    642         case TASTART:
    643             if (style()->isLeftToRightDirection())
    644                 updateLogicalWidthForLeftAlignedBlock(style()->isLeftToRightDirection(), trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth);
    645             else
    646                 updateLogicalWidthForRightAlignedBlock(style()->isLeftToRightDirection(), trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth);
    647             break;
    648         case TAEND:
    649             if (style()->isLeftToRightDirection())
    650                 updateLogicalWidthForRightAlignedBlock(style()->isLeftToRightDirection(), trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth);
    651             else
    652                 updateLogicalWidthForLeftAlignedBlock(style()->isLeftToRightDirection(), trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth);
    653             break;
    654     }
     659    updateLogicalWidthForAlignment(textAlign, trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth, expansionOpportunityCount);
    655660
    656661    computeExpansionForJustifiedText(firstRun, trailingSpaceRun, expansionOpportunities, expansionOpportunityCount, totalLogicalWidth, availableLogicalWidth);
    … …  
    713718        // position as though we were an inline. Set |staticInlinePosition| and |staticBlockPosition| on the relative positioned
    714719        // inline so that we can obtain the value later.
    715         toRenderInline(containerBlock)->layer()->setStaticInlinePosition(block->startOffsetForLine(blockHeight, false));
     720        toRenderInline(containerBlock)->layer()->setStaticInlinePosition(block->startAlignedOffsetForLine(blockHeight, false));
    716721        toRenderInline(containerBlock)->layer()->setStaticBlockPosition(blockHeight);
    717722    }
    718723
    719724    if (child->style()->isOriginalDisplayInlineType())
    720         child->layer()->setStaticInlinePosition(block->startOffsetForLine(blockHeight, false));
     725        child->layer()->setStaticInlinePosition(block->startAlignedOffsetForLine(blockHeight, false));
    721726    else
    722         child->layer()->setStaticInlinePosition(block->borderAndPaddingStart());
     727        child->layer()->setStaticInlinePosition(block->startAlignedOffsetForBlock(blockHeight, false));
    723728    child->layer()->setStaticBlockPosition(blockHeight);
    724729}
    … …  
    20452050            bool isInlineType = box->style()->isOriginalDisplayInlineType();
    20462051            if (!isInlineType)
    2047                 box->layer()->setStaticInlinePosition(m_block->borderAndPaddingStart());
     2052                box->layer()->setStaticInlinePosition(m_block->startAlignedOffsetForBlock(m_block->logicalHeight(), false));
    20482053            else  {
    20492054                // If our original display was an INLINE type, then we can go ahead
    … …  
    25902595}
    25912596
    2592 }
     2597LayoutUnit RenderBlock::startAlignedOffsetForBlock(LayoutUnit position, bool firstLine)
     2598{
     2599    ETextAlign textAlign = style()->textAlign();
     2600
     2601    if (textAlign == TAAUTO)
     2602        return borderAndPaddingStart();
     2603
     2604    LayoutUnit lineOffset = startAlignedOffsetForLine(position, firstLine);
     2605
     2606    // For block flows, unlike inlines, the offset is given from the point at which the
     2607    // flow starts. So if the flow is RTL, flip the offset to represent units from the right.
     2608    if (!style()->isLeftToRightDirection())
     2609        lineOffset = logicalWidth() - lineOffset;
     2610    return lineOffset;
     2611}
     2612
     2613LayoutUnit RenderBlock::startAlignedOffsetForLine(LayoutUnit position, bool firstLine)
     2614{
     2615    ETextAlign textAlign = style()->textAlign();
     2616
     2617    if (textAlign == TAAUTO)
     2618        return startOffsetForLine(position, firstLine);
     2619
     2620    // updateLogicalWidthForAlignment() handles the direction of the block so no need to consider it here
     2621    float logicalLeft;
     2622    float availableLogicalWidth;
     2623    logicalLeft = logicalLeftOffsetForLine(logicalHeight(), false);
     2624    availableLogicalWidth = logicalRightOffsetForLine(logicalHeight(), false) - logicalLeft;
     2625    float totalLogicalWidth;
     2626    updateLogicalWidthForAlignment(textAlign, 0l, logicalLeft, totalLogicalWidth, availableLogicalWidth, 0);
     2627    return logicalLeft;
     2628}
     2629
     2630}
Note: See TracChangeset for help on using the changeset viewer.