Changeset 53525 in webkit


Ignore:
Timestamp:
Jan 19, 2010, 11:46:52 PM (15 years ago)
Author:
rolandsteiner@chromium.org
Message:

Bug 33266 - WebCore::InlineFlowBox::determineSpacingForFlowBoxes ReadAV@NULL (43c64e8abbda6766e5f5edbd254c2d57)
(https://bugs.webkit.org/show_bug.cgi?id=33266)

Reviewed by Dan Bernstein.

WebCore:

Ruby did not handle malformed cases correctly when the ruby base was in
block flow. Changed the code to handle all possible cases.
Also, added some simplification methods to RenderBlock.

Tests: fast/ruby/ruby-illegal-1.html

fast/ruby/ruby-illegal-2.html
fast/ruby/ruby-illegal-3.html
fast/ruby/ruby-illegal-4.html
fast/ruby/ruby-illegal-5.html
fast/ruby/ruby-illegal-6.html
fast/ruby/ruby-illegal-7.html
fast/ruby/ruby-illegal-combined.html
fast/ruby/rubyDOM-insert-rt-block-1.html
fast/ruby/rubyDOM-insert-rt-block-2.html
fast/ruby/rubyDOM-insert-rt-block-3.html
fast/ruby/rubyDOM-remove-rt-block-1.html
fast/ruby/rubyDOM-remove-rt-block-2.html
fast/ruby/rubyDOM-remove-rt-block-3.html

  • rendering/RenderBlock.cpp:

(WebCore::RenderBlock::moveAllChildrenTo): useful for anonymous block manipulation
(WebCore::RenderBlock::removeChild): making use of moveAllChildrenTo

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

(WebCore::RenderRubyBase::hasOnlyWrappedInlineChildren):
(WebCore::RenderRubyBase::moveChildren):
(WebCore::RenderRubyBase::moveInlineChildren):
(WebCore::RenderRubyBase::moveBlockChildren):
(WebCore::RenderRubyBase::mergeBlockChildren):

  • rendering/RenderRubyBase.h:
  • rendering/RenderRubyRun.cpp:

(WebCore::RenderRubyRun::addChild):
(WebCore::RenderRubyRun::removeChild):

LayoutTests:

Layout tests for ruby with malformed HTML.
Split up in individual tests, as well ass added a single combined test
(whose resulting render tree is probably completely different from what
you'd expect), since that combined test showed additional issues not
covered by the individual tests.

  • fast/ruby/ruby-illegal-1-expected.txt: Added.
  • fast/ruby/ruby-illegal-1.html: Added.
  • fast/ruby/ruby-illegal-2-expected.txt: Added.
  • fast/ruby/ruby-illegal-2.html: Added.
  • fast/ruby/ruby-illegal-3-expected.txt: Added.
  • fast/ruby/ruby-illegal-3.html: Added.
  • fast/ruby/ruby-illegal-4-expected.txt: Added.
  • fast/ruby/ruby-illegal-4.html: Added.
  • fast/ruby/ruby-illegal-5-expected.txt: Added.
  • fast/ruby/ruby-illegal-5.html: Added.
  • fast/ruby/ruby-illegal-6-expected.txt: Added.
  • fast/ruby/ruby-illegal-6.html: Added.
  • fast/ruby/ruby-illegal-7-expected.txt: Added.
  • fast/ruby/ruby-illegal-7.html: Added.
  • fast/ruby/ruby-illegal-combined-expected.txt: Added.
  • fast/ruby/ruby-illegal-combined.html: Added.
  • fast/ruby/ruby-illegal-expected.txt: Removed.
  • fast/ruby/ruby-illegal.html: Removed.
  • fast/ruby/rubyDOM-insert-rt-block-1-expected.txt: Added.
  • fast/ruby/rubyDOM-insert-rt-block-1.html: Added.
  • fast/ruby/rubyDOM-insert-rt-block-2-expected.txt: Added.
  • fast/ruby/rubyDOM-insert-rt-block-2.html: Added.
  • fast/ruby/rubyDOM-insert-rt-block-3-expected.txt: Added.
  • fast/ruby/rubyDOM-insert-rt-block-3.html: Added.
  • fast/ruby/rubyDOM-remove-rt-block-1-expected.txt: Added.
  • fast/ruby/rubyDOM-remove-rt-block-1.html: Added.
  • fast/ruby/rubyDOM-remove-rt-block-2-expected.txt: Added.
  • fast/ruby/rubyDOM-remove-rt-block-2.html: Added.
  • fast/ruby/rubyDOM-remove-rt-block-3-expected.txt: Added.
  • fast/ruby/rubyDOM-remove-rt-block-3.html: Added.
Location:
trunk
Files:
28 added
2 deleted
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r53512 r53525  
     12010-01-20  Roland Steiner  <rolandsteiner@chromium.org>
     2
     3        Reviewed by Dan Bernstein.
     4
     5        Bug 33266 - WebCore::InlineFlowBox::determineSpacingForFlowBoxes ReadAV@NULL (43c64e8abbda6766e5f5edbd254c2d57)
     6        (https://bugs.webkit.org/show_bug.cgi?id=33266)
     7       
     8        Layout tests for ruby with malformed HTML.
     9        Split up in individual tests, as well ass added a single combined test
     10        (whose resulting render tree is probably completely different from what
     11        you'd expect), since that combined test showed additional issues not
     12        covered by the individual tests.
     13
     14        * fast/ruby/ruby-illegal-1-expected.txt: Added.
     15        * fast/ruby/ruby-illegal-1.html: Added.
     16        * fast/ruby/ruby-illegal-2-expected.txt: Added.
     17        * fast/ruby/ruby-illegal-2.html: Added.
     18        * fast/ruby/ruby-illegal-3-expected.txt: Added.
     19        * fast/ruby/ruby-illegal-3.html: Added.
     20        * fast/ruby/ruby-illegal-4-expected.txt: Added.
     21        * fast/ruby/ruby-illegal-4.html: Added.
     22        * fast/ruby/ruby-illegal-5-expected.txt: Added.
     23        * fast/ruby/ruby-illegal-5.html: Added.
     24        * fast/ruby/ruby-illegal-6-expected.txt: Added.
     25        * fast/ruby/ruby-illegal-6.html: Added.
     26        * fast/ruby/ruby-illegal-7-expected.txt: Added.
     27        * fast/ruby/ruby-illegal-7.html: Added.
     28        * fast/ruby/ruby-illegal-combined-expected.txt: Added.
     29        * fast/ruby/ruby-illegal-combined.html: Added.
     30        * fast/ruby/ruby-illegal-expected.txt: Removed.
     31        * fast/ruby/ruby-illegal.html: Removed.
     32        * fast/ruby/rubyDOM-insert-rt-block-1-expected.txt: Added.
     33        * fast/ruby/rubyDOM-insert-rt-block-1.html: Added.
     34        * fast/ruby/rubyDOM-insert-rt-block-2-expected.txt: Added.
     35        * fast/ruby/rubyDOM-insert-rt-block-2.html: Added.
     36        * fast/ruby/rubyDOM-insert-rt-block-3-expected.txt: Added.
     37        * fast/ruby/rubyDOM-insert-rt-block-3.html: Added.
     38        * fast/ruby/rubyDOM-remove-rt-block-1-expected.txt: Added.
     39        * fast/ruby/rubyDOM-remove-rt-block-1.html: Added.
     40        * fast/ruby/rubyDOM-remove-rt-block-2-expected.txt: Added.
     41        * fast/ruby/rubyDOM-remove-rt-block-2.html: Added.
     42        * fast/ruby/rubyDOM-remove-rt-block-3-expected.txt: Added.
     43        * fast/ruby/rubyDOM-remove-rt-block-3.html: Added.
     44
    1452010-01-17  Jon Honeycutt  <jhoneycutt@apple.com>
    246
  • trunk/WebCore/ChangeLog

    r53524 r53525  
     12010-01-20  Roland Steiner  <rolandsteiner@chromium.org>
     2
     3        Reviewed by Dan Bernstein.
     4
     5        Bug 33266 - WebCore::InlineFlowBox::determineSpacingForFlowBoxes ReadAV@NULL (43c64e8abbda6766e5f5edbd254c2d57)
     6        (https://bugs.webkit.org/show_bug.cgi?id=33266)
     7       
     8        Ruby did not handle malformed cases correctly when the ruby base was in
     9        block flow. Changed the code to handle all possible cases.
     10        Also, added some simplification methods to RenderBlock.
     11
     12        Tests: fast/ruby/ruby-illegal-1.html
     13               fast/ruby/ruby-illegal-2.html
     14               fast/ruby/ruby-illegal-3.html
     15               fast/ruby/ruby-illegal-4.html
     16               fast/ruby/ruby-illegal-5.html
     17               fast/ruby/ruby-illegal-6.html
     18               fast/ruby/ruby-illegal-7.html
     19               fast/ruby/ruby-illegal-combined.html
     20               fast/ruby/rubyDOM-insert-rt-block-1.html
     21               fast/ruby/rubyDOM-insert-rt-block-2.html
     22               fast/ruby/rubyDOM-insert-rt-block-3.html
     23               fast/ruby/rubyDOM-remove-rt-block-1.html
     24               fast/ruby/rubyDOM-remove-rt-block-2.html
     25               fast/ruby/rubyDOM-remove-rt-block-3.html
     26
     27        * rendering/RenderBlock.cpp:
     28        (WebCore::RenderBlock::moveAllChildrenTo): useful for anonymous block manipulation
     29        (WebCore::RenderBlock::removeChild): making use of moveAllChildrenTo
     30        * rendering/RenderBlock.h:
     31        * rendering/RenderRubyBase.cpp:
     32        (WebCore::RenderRubyBase::hasOnlyWrappedInlineChildren):
     33        (WebCore::RenderRubyBase::moveChildren):
     34        (WebCore::RenderRubyBase::moveInlineChildren):
     35        (WebCore::RenderRubyBase::moveBlockChildren):
     36        (WebCore::RenderRubyBase::mergeBlockChildren):
     37        * rendering/RenderRubyBase.h:
     38        * rendering/RenderRubyRun.cpp:
     39        (WebCore::RenderRubyRun::addChild):
     40        (WebCore::RenderRubyRun::removeChild):
     41
    1422010-01-19  Dan Bernstein  <mitz@apple.com>
    243
  • trunk/WebCore/rendering/RenderBlock.cpp

    r53420 r53525  
    412412}
    413413
     414void RenderBlock::moveAllChildrenTo(RenderObject* to, RenderObjectChildList* toChildList)
     415{
     416    RenderObject* nextChild = children()->firstChild();
     417    while (nextChild) {
     418        RenderObject* child = nextChild;
     419        nextChild = child->nextSibling();
     420        toChildList->appendChildNode(to, children()->removeChildNode(this, child, false), false);
     421    }
     422}
     423
     424void RenderBlock::moveAllChildrenTo(RenderObject* to, RenderObjectChildList* toChildList, RenderObject* beforeChild)
     425{
     426    ASSERT(!beforeChild || to == beforeChild->parent());
     427    if (!beforeChild) {
     428        moveAllChildrenTo(to, toChildList);
     429        return;
     430    }
     431    RenderObject* nextChild = children()->firstChild();
     432    while (nextChild) {
     433        RenderObject* child = nextChild;
     434        nextChild = child->nextSibling();
     435        toChildList->insertChildNode(to, children()->removeChildNode(this, child, false), beforeChild, false);
     436    }
     437}
     438
    414439void RenderBlock::makeChildrenNonInline(RenderObject *insertionPoint)
    415440{   
     
    518543        // the |prev| block.
    519544        prev->setNeedsLayoutAndPrefWidthsRecalc();
    520         RenderObject* o = next->firstChild();
    521    
    522545        RenderBlock* nextBlock = toRenderBlock(next);
    523546        RenderBlock* prevBlock = toRenderBlock(prev);
    524         while (o) {
    525             RenderObject* no = o;
    526             o = no->nextSibling();
    527             nextBlock->moveChildTo(prevBlock, prevBlock->children(), no);
    528         }
    529  
     547        nextBlock->moveAllChildrenTo(prevBlock, prevBlock->children());
     548        // Delete the now-empty block's lines and nuke it.
    530549        nextBlock->deleteLineBoxTree();
    531        
    532         // Nuke the now-empty block.
    533         next->destroy();
     550        nextBlock->destroy();
    534551    }
    535552
     
    544561        RenderBlock* anonBlock = toRenderBlock(children()->removeChildNode(this, child, false));
    545562        setChildrenInline(true);
    546         RenderObject* o = anonBlock->firstChild();
    547         while (o) {
    548             RenderObject* no = o;
    549             o = no->nextSibling();
    550             anonBlock->moveChildTo(this, children(), no);
    551         }
    552 
     563        anonBlock->moveAllChildrenTo(this, children());
    553564        // Delete the now-empty block's lines and nuke it.
    554565        anonBlock->deleteLineBoxTree();
  • trunk/WebCore/rendering/RenderBlock.h

    r53420 r53525  
    142142    void moveChildTo(RenderObject* to, RenderObjectChildList* toChildList, RenderObject* child);
    143143    void moveChildTo(RenderObject* to, RenderObjectChildList* toChildList, RenderObject* beforeChild, RenderObject* child);
     144    void moveAllChildrenTo(RenderObject* to, RenderObjectChildList* toChildList);
     145    void moveAllChildrenTo(RenderObject* to, RenderObjectChildList* toChildList, RenderObject* beforeChild);
    144146
    145147    int maxTopPosMargin() const { return m_maxMargin ? m_maxMargin->m_topPos : MaxMargin::topPosDefault(this); }
     
    516518
    517519    mutable int m_lineHeight;
     520   
     521    // RenderRubyBase objects need to be able to split and merge, moving their children around
     522    // (calling moveChildTo, moveAllChildrenTo, and makeChildrenNonInline).
     523    friend class RenderRubyBase;
    518524};
    519525
  • trunk/WebCore/rendering/RenderRubyBase.cpp

    r50397 r53525  
    4949}
    5050
    51 void RenderRubyBase::splitToLeft(RenderBlock* leftBase, RenderObject* beforeChild)
     51bool RenderRubyBase::hasOnlyWrappedInlineChildren(RenderObject* beforeChild) const
     52{
     53    // Tests whether all children in the base before beforeChild are either floated/positioned,
     54    // or inline objects wrapped in anonymous blocks.
     55    // Note that beforeChild may be 0, in which case all children are looked at.
     56    for (RenderObject* child = firstChild(); child != beforeChild; child = child->nextSibling()) {
     57        if (!child->isFloatingOrPositioned() && !(child->isAnonymousBlock() && child->childrenInline()))
     58            return false;
     59    }
     60    return true;
     61}
     62
     63void RenderRubyBase::moveChildren(RenderRubyBase* toBase, RenderObject* fromBeforeChild)
    5264{
    5365    // This function removes all children that are before (!) beforeChild
    54     // and appends them to leftBase.
    55     ASSERT(leftBase);
     66    // and appends them to toBase.
     67    ASSERT(toBase);
     68   
     69    // First make sure that beforeChild (if set) is indeed a direct child of this.
     70    // Inline children might be wrapped in an anonymous block if there's a continuation.
     71    // Theoretically, in ruby bases, this can happen with only the first such a child,
     72    // so it should be OK to just climb the tree.
     73    while (fromBeforeChild && fromBeforeChild->parent() != this)
     74        fromBeforeChild = fromBeforeChild->parent();
    5675
    57     // First make sure that beforeChild (if set) is indeed a direct child of this.
    58     // Fallback: climb up the tree to make sure. This may result in somewhat incorrect rendering.
    59     // FIXME: Can this happen? Is there a better/more correct way to solve this?
    60     ASSERT(!beforeChild || beforeChild->parent() == this);
    61     while (beforeChild && beforeChild->parent() != this)
    62         beforeChild = beforeChild->parent();
     76    if (childrenInline())
     77        moveInlineChildren(toBase, fromBeforeChild);
     78    else
     79        moveBlockChildren(toBase, fromBeforeChild);
    6380
    64     RenderObject* child = firstChild();
    65     while (child != beforeChild) {
    66         RenderObject* nextChild = child->nextSibling();
    67         moveChildTo(leftBase, leftBase->children(), child);
    68         child = nextChild;
    69     }
     81    setNeedsLayoutAndPrefWidthsRecalc();
     82    toBase->setNeedsLayoutAndPrefWidthsRecalc();
    7083}
    7184
    72 void RenderRubyBase::mergeWithRight(RenderBlock* rightBase)
     85void RenderRubyBase::moveInlineChildren(RenderRubyBase* toBase, RenderObject* fromBeforeChild)
    7386{
    74     // This function removes all children and prepends (!) them to rightBase.
    75     ASSERT(rightBase);
     87    RenderBlock* toBlock;
    7688
    77     RenderObject* firstPos = rightBase->firstChild();
    78     RenderObject* child = lastChild();
    79     while (child) {
    80         moveChildTo(rightBase, rightBase->children(), firstPos, child);
    81         firstPos = child;
    82         child = lastChild();
     89    if (toBase->childrenInline()) {
     90        // The standard and easy case: move the children into the target base
     91        toBlock = toBase;
     92    } else {
     93        // We need to wrap the inline objects into an anonymous block.
     94        // If toBase has a suitable block, we re-use it, otherwise create a new one.
     95        RenderObject* lastChild = toBase->lastChild();
     96        if (lastChild && lastChild->isAnonymousBlock() && lastChild->childrenInline())
     97            toBlock = toRenderBlock(lastChild);
     98        else {
     99            toBlock = toBase->createAnonymousBlock();
     100            toBase->children()->appendChildNode(toBase, toBlock);
     101        }
    83102    }
     103    // Move our inline children into the target block we determined above.
     104    for (RenderObject* child = firstChild(); child != fromBeforeChild; child = firstChild())
     105        moveChildTo(toBlock, toBlock->children(), child);
     106}
     107
     108void RenderRubyBase::moveBlockChildren(RenderRubyBase* toBase, RenderObject* fromBeforeChild)
     109{
     110    if (toBase->childrenInline()) {
     111        // First check whether we move only wrapped inline objects.
     112        if (hasOnlyWrappedInlineChildren(fromBeforeChild)) {
     113            // The reason why the base is in block flow must be after beforeChild.
     114            // We therefore can extract the inline objects and move them to toBase.
     115            for (RenderObject* child = firstChild(); child != fromBeforeChild; child = firstChild()) {
     116                if (child->isAnonymousBlock()) {
     117                    RenderBlock* anonBlock = toRenderBlock(child);
     118                    ASSERT(anonBlock->childrenInline());
     119                    ASSERT(!anonBlock->inlineContinuation());
     120                    anonBlock->moveAllChildrenTo(toBase, toBase->children());
     121                    anonBlock->deleteLineBoxTree();
     122                    anonBlock->destroy();
     123                } else {
     124                    ASSERT(child->isFloatingOrPositioned());
     125                    moveChildTo(toBase, toBase->children(), child);
     126                }
     127            }
     128        } else {
     129            // Moving block children -> have to set toBase as block flow
     130            toBase->makeChildrenNonInline();
     131            // Move children, potentially collapsing anonymous block wrappers.
     132            mergeBlockChildren(toBase, fromBeforeChild);
     133
     134            // Now we need to check if the leftover children are all inline.
     135            // If so, make this base inline again.
     136            if (hasOnlyWrappedInlineChildren()) {
     137                RenderObject* next = 0;
     138                for (RenderObject* child = firstChild(); child; child = next) {
     139                    next = child->nextSibling();
     140                    if (child->isFloatingOrPositioned())
     141                        continue;
     142                    ASSERT(child->isAnonymousBlock());
     143
     144                    RenderBlock* anonBlock = toRenderBlock(child);
     145                    ASSERT(anonBlock->childrenInline());
     146                    ASSERT(!anonBlock->inlineContinuation());
     147                    // Move inline children out of anonymous block.
     148                    anonBlock->moveAllChildrenTo(this, children(), anonBlock);
     149                    anonBlock->deleteLineBoxTree();
     150                    anonBlock->destroy();
     151                }
     152                setChildrenInline(true);
     153            }
     154        }
     155    } else
     156        mergeBlockChildren(toBase, fromBeforeChild);
     157}
     158
     159void RenderRubyBase::mergeBlockChildren(RenderRubyBase* toBase, RenderObject* fromBeforeChild)
     160{
     161    // This function removes all children that are before fromBeforeChild and appends them to toBase.
     162    ASSERT(!childrenInline());
     163    ASSERT(toBase);
     164    ASSERT(!toBase->childrenInline());
     165
     166    // Quick check whether we have anything to do, to simplify the following code.
     167    if (fromBeforeChild != firstChild())
     168        return;
     169
     170    // If an anonymous block would be put next to another such block, then merge those.
     171    RenderObject* firstChildHere = firstChild();
     172    RenderObject* lastChildThere = toBase->lastChild();
     173    if (firstChildHere && firstChildHere->isAnonymousBlock() && firstChildHere->childrenInline()
     174            && lastChildThere && lastChildThere->isAnonymousBlock() && lastChildThere->childrenInline()) {           
     175        RenderBlock* anonBlockHere = toRenderBlock(firstChildHere);
     176        RenderBlock* anonBlockThere = toRenderBlock(lastChildThere);
     177        anonBlockHere->moveAllChildrenTo(anonBlockThere, anonBlockThere->children());
     178        anonBlockHere->deleteLineBoxTree();
     179        anonBlockHere->destroy();
     180    }
     181    // Move all remaining children normally.
     182    for (RenderObject* child = firstChild(); child != fromBeforeChild; child = firstChild())
     183        moveChildTo(toBase, toBase->children(), child);
    84184}
    85185
  • trunk/WebCore/rendering/RenderRubyBase.h

    r50397 r53525  
    4747    virtual bool isChildAllowed(RenderObject*, RenderStyle*) const;
    4848
    49     void splitToLeft(RenderBlock* leftBase, RenderObject* beforeChild);
    50     void mergeWithRight(RenderBlock* rightBase);
     49private:
     50    bool hasOnlyWrappedInlineChildren(RenderObject* beforeChild = 0) const;
     51
     52    void moveChildren(RenderRubyBase* toBase, RenderObject* fromBeforeChild = 0);
     53    void moveInlineChildren(RenderRubyBase* toBase, RenderObject* fromBeforeChild = 0);
     54    void moveBlockChildren(RenderRubyBase* toBase, RenderObject* fromBeforeChild = 0);
     55    void mergeBlockChildren(RenderRubyBase* toBase, RenderObject* fromBeforeChild = 0);
     56   
     57    // Allow RenderRubyRun to manipulate the children within ruby bases.
     58    friend class RenderRubyRun;
    5159};
    5260
  • trunk/WebCore/rendering/RenderRubyRun.cpp

    r51169 r53525  
    149149            ruby->addChild(newRun, this);
    150150            newRun->addChild(child);
    151             rubyBaseSafe()->splitToLeft(newRun->rubyBaseSafe(), beforeChild);
     151            rubyBaseSafe()->moveChildren(newRun->rubyBaseSafe(), beforeChild);
    152152        }
    153153    } else {
     
    171171            RenderRubyRun* rightRun = static_cast<RenderRubyRun*>(rightNeighbour);
    172172            ASSERT(rightRun->hasRubyBase());
    173             base->mergeWithRight(rightRun->rubyBaseSafe());
     173            RenderRubyBase* rightBase = rightRun->rubyBaseSafe();
     174            // Collect all children in a single base, then swap the bases.
     175            rightBase->moveChildren(base);
     176            moveChildTo(rightRun, rightRun->children(), base);
     177            rightRun->moveChildTo(this, children(), rightBase);
    174178            // The now empty ruby base will be removed below.
    175179        }
Note: See TracChangeset for help on using the changeset viewer.