Changeset 94492 in webkit


Ignore:
Timestamp:
Sep 3, 2011 11:28:57 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:

When an inline 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.

Tests: fast/css/bug4860-absolute-block-child-does-not-inherit-alignment.html

  • Ensure positioned block elements inherit alignment. fast/css/bug4860-absolute-inline-child-inherits-alignment.html
  • Ensure positioned inline elements inherit alignment. fast/inline/absolute-positioned-inline-in-centred-block.html
  • Ensure positioned inline element that's the sole or first child of a rendered block obeys parents alignment. fast/inline/absolute-positioned-block-in-centred-block.html
  • As above, but a positioned block should not inherit alignment.
  • 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::startAlignedOffsetForLine): New function, find the aligned offset using updateLogicalWidthForAlignment

LayoutTests:

  • fast/css/bug4860-absolute-block-child-does-not-inherit-alignment-expected.png: Added.
  • fast/css/bug4860-absolute-block-child-does-not-inherit-alignment-expected.txt: Added.
  • fast/css/bug4860-absolute-block-child-does-not-inherit-alignment.html: Added.
  • fast/css/bug4860-absolute-inline-child-inherits-alignment-expected.png: Added.
  • fast/css/bug4860-absolute-inline-child-inherits-alignment-expected.txt: Added.
  • fast/css/bug4860-absolute-inline-child-inherits-alignment.html: Added.
  • 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.
  • fast/css/absolute-child-with-percent-height-inside-relative-parent-expected.txt:
  • platform/chromium-win/fast/css/absolute-child-with-percent-height-inside-relative-parent-expected.png: This test expected the wrong alignment of the red block - it should be centred, not aligned to the left.
Location:
trunk
Files:
12 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r94482 r94492  
     12011-08-27  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/css/bug4860-absolute-block-child-does-not-inherit-alignment-expected.png: Added.
     9        * fast/css/bug4860-absolute-block-child-does-not-inherit-alignment-expected.txt: Added.
     10        * fast/css/bug4860-absolute-block-child-does-not-inherit-alignment.html: Added.
     11        * fast/css/bug4860-absolute-inline-child-inherits-alignment-expected.png: Added.
     12        * fast/css/bug4860-absolute-inline-child-inherits-alignment-expected.txt: Added.
     13        * fast/css/bug4860-absolute-inline-child-inherits-alignment.html: Added.
     14        * fast/inline/absolute-positioned-block-in-centred-block-expected.png: Added.
     15        * fast/inline/absolute-positioned-block-in-centred-block-expected.txt: Added.
     16        * fast/inline/absolute-positioned-block-in-centred-block.html: Added.
     17        * fast/inline/absolute-positioned-inline-in-centred-block-expected.png: Added.
     18        * fast/inline/absolute-positioned-inline-in-centred-block-expected.txt: Added.
     19        * fast/inline/absolute-positioned-inline-in-centred-block.html: Added.
     20        * platform/chromium-linux/fast/repaint/block-layout-inline-children-float-positioned-expected.png:
     21        * platform/chromium-linux/fast/repaint/block-layout-inline-children-float-positioned-expected.txt:
     22          Both of these two tests were based on the incorrect behaviour of ignoring the alignment specified
     23          by the container of an element absolute position when there was no text preceding the element.
     24          The updated results agree with Firefox and IE.
     25        * fast/css/absolute-child-with-percent-height-inside-relative-parent-expected.txt:
     26        * platform/chromium-win/fast/css/absolute-child-with-percent-height-inside-relative-parent-expected.png:
     27          This test expected the wrong alignment of the red block - it should be centred, not aligned to the left.
     28
     292011-07-25  Robert Hogan  <robert@webkit.org>
     30
     31        div align="center" rendering problem
     32        https://bugs.webkit.org/show_bug.cgi?id=4860
     33
     34        Reviewed by David Hyatt.
     35
     36        * fast/inline/absolute-positioned-block-in-centred-block-expected.png: Added.
     37        * fast/inline/absolute-positioned-block-in-centred-block-expected.txt: Added.
     38        * fast/inline/absolute-positioned-block-in-centred-block.html: Added.
     39        * fast/inline/absolute-positioned-inline-in-centred-block-expected.png: Added.
     40        * fast/inline/absolute-positioned-inline-in-centred-block-expected.txt: Added.
     41        * fast/inline/absolute-positioned-inline-in-centred-block.html: Added.
     42        * platform/chromium-linux/fast/repaint/block-layout-inline-children-float-positioned-expected.png:
     43        * platform/chromium-linux/fast/repaint/block-layout-inline-children-float-positioned-expected.txt:
     44          Both of these two tests were based on the incorrect behaviour of ignoring the alignment specified
     45          by the container of an element absolute position when there was no text preceding the element.
     46          The updated results agree with Firefox and IE.
     47
    1482011-09-03  Yuta Kitamura  <yutak@chromium.org>
    249

  • trunk/LayoutTests/fast/css/absolute-child-with-percent-height-inside-relative-parent-expected.txt

    r91533 r94492  
    77layer at (87,8) size 627x230
    88  RenderBlock (relative positioned) {DIV} at (79,0) size 627x230 [bgcolor=#0000FF]
    9 layer at (87,8) size 501x230
    10   RenderBlock (positioned) {DIV} at (0,0) size 501x230 [bgcolor=#FF0000]
     9layer at (154,8) size 501x230
     10  RenderBlock (positioned) {DIV} at (67,0) size 501x230 [bgcolor=#FF0000]
    1111    RenderImage {IMG} at (51,0) size 400x230
    1212    RenderText {#text} at (0,0) size 0x0

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

    r93621 r94492  
    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

    r94491 r94492  
     12011-08-27  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        When an inline element with absolute position was the sole or first child of a render block with
     9        centred alignment, it wasn't obeying its parent's alignment. However it would obey the
     10        alignment if it was preceded by some text. The problem was that the element's render object
     11        was getting skipped as leading white space, so it was not included in a normal line block in
     12        a bidi run. Instead, its position was getting set by RenderBlockLineLayout::setStaticPositions()
     13        which does not pay attention to alignment. Preceding the element with some text allowed the object
     14        to get included in a Bidi run and so get a linebox which would get properly aligned.
     15
     16        The fix is to get RenderBlockLineLayout::setStaticPositions() to obey the alignment specified by
     17        the object's container. This allows WebKit to get the same result on the test as Firefox and IE.
     18        Opera has the same bug as unpatched WebKit.
     19
     20        Tests: fast/css/bug4860-absolute-block-child-does-not-inherit-alignment.html
     21               - Ensure positioned block elements inherit alignment.
     22               fast/css/bug4860-absolute-inline-child-inherits-alignment.html
     23               - Ensure positioned inline elements inherit alignment.
     24               fast/inline/absolute-positioned-inline-in-centred-block.html
     25               - Ensure positioned inline element that's the sole or first child of a rendered block
     26                 obeys parents alignment.
     27               fast/inline/absolute-positioned-block-in-centred-block.html
     28               - As above, but a positioned block should not inherit alignment.
     29
     30        * rendering/RenderBlock.h:
     31        * rendering/RenderBlockLineLayout.cpp:
     32        (WebCore::RenderBlock::updateLogicalWidthForAlignment):
     33        (WebCore::RenderBlock::computeInlineDirectionPositionsForLine): Move the alignment check to updateLogicalWidthForAlignment.
     34        (WebCore::setStaticPositions): use startAlignedOffsetForLine and use startAlignedOffsetForBlock
     35        (WebCore::RenderBlock::startAlignedOffsetForLine): New function, find the aligned offset using updateLogicalWidthForAlignment
     36
    1372011-09-03  Andreas Kling  <kling@webkit.org>
    238

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

    r93627 r94492  
    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(RenderBox* child, LayoutUnit position, bool firstLine);
    132133    LayoutUnit textIndentOffset() const;
    133134
    … …  
    224225    LayoutUnit collapsedMarginAfterForChild(RenderBox* child) const;
    225226
     227    void updateLogicalWidthForAlignment(const ETextAlign&, BidiRun* trailingSpaceRun, float& logicalLeft, float& totalLogicalWidth, float& availableLogicalWidth, int expansionOpportunityCount);
     228
    226229    virtual void updateFirstLetter();
    227230

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

    r94084 r94492  
    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);
    … …  
    712717        // position as though we were an inline. Set |staticInlinePosition| and |staticBlockPosition| on the relative positioned
    713718        // inline so that we can obtain the value later.
    714         toRenderInline(containerBlock)->layer()->setStaticInlinePosition(block->startOffsetForLine(blockHeight, false));
     719        toRenderInline(containerBlock)->layer()->setStaticInlinePosition(block->startAlignedOffsetForLine(child, blockHeight, false));
    715720        toRenderInline(containerBlock)->layer()->setStaticBlockPosition(blockHeight);
    716721    }
    717722
    718723    if (child->style()->isOriginalDisplayInlineType())
    719         child->layer()->setStaticInlinePosition(block->startOffsetForLine(blockHeight, false));
     724        child->layer()->setStaticInlinePosition(block->startAlignedOffsetForLine(child, blockHeight, false));
    720725    else
    721726        child->layer()->setStaticInlinePosition(block->borderAndPaddingStart());
    … …  
    25992604}
    26002605
    2601 }
     2606LayoutUnit RenderBlock::startAlignedOffsetForLine(RenderBox* child, LayoutUnit position, bool firstLine)
     2607{
     2608    ETextAlign textAlign = style()->textAlign();
     2609
     2610    if (textAlign == TAAUTO)
     2611        return startOffsetForLine(position, firstLine);
     2612
     2613    // updateLogicalWidthForAlignment() handles the direction of the block so no need to consider it here
     2614    float logicalLeft;
     2615    float availableLogicalWidth;
     2616    logicalLeft = logicalLeftOffsetForLine(logicalHeight(), false);
     2617    availableLogicalWidth = logicalRightOffsetForLine(logicalHeight(), false) - logicalLeft;
     2618    float totalLogicalWidth = logicalWidthForChild(child);
     2619    updateLogicalWidthForAlignment(textAlign, 0l, logicalLeft, totalLogicalWidth, availableLogicalWidth, 0);
     2620    return logicalLeft;
     2621}
     2622
     2623}
Note: See TracChangeset for help on using the changeset viewer.