Changeset 85355 in webkit


Ignore:
Timestamp:
Apr 29, 2011 1:38:26 PM (13 years ago)
Author:
inferno@chromium.org
Message:

2011-04-29 Abhishek Arya <inferno@chromium.org>

Reviewed by Dave Hyatt.

Allow only first table caption and destroy the remaining ones.
https://bugs.webkit.org/show_bug.cgi?id=58249

Previously, we were only laying out the first table caption.
However Table::layout didn't mark the other ones as not needing
layout. So after table layout completes, table is marked as not
needing layout with its other table caption still needing layout.
This causes incorrect layout root calculations and set it to a
node which is already getting deleted.

Tests: fast/table/dynamic-caption-add-before-child.xhtml

fast/table/dynamic-caption-add-remove-before-child.xhtml
fast/table/multiple-captions-crash.xhtml
fast/table/multiple-captions-crash2.xhtml
fast/table/multiple-captions-display.xhtml

  • rendering/RenderTable.cpp: (WebCore::RenderTable::addChild): when new caption or a before child caption is added, we need to explicitly trigger section recalc or otherwise layout won't catch it. (WebCore::RenderTable::removeChild): when child to be removed is m_caption, make sure to trigger style recalc on the table. (WebCore::RenderTable::recalcCaption): code to destroy captions other than the first one. (WebCore::RenderTable::recalcSections): call recalcCaption helper. Store the next sibling early since child can get destroyed in recalcCaption.
  • rendering/RenderTable.h:

2011-04-21 Abhishek Arya <inferno@chromium.org>

Reviewed by Dave Hyatt.

Tests that we do not crash on

ASSERT(!m_layoutRoot->container()
!m_layoutRoot->container()->needsLayout())

when a table has two or more captions.
https://bugs.webkit.org/show_bug.cgi?id=58249

  • fast/table/dynamic-caption-add-before-child.xhtml: Added. Tests that before child caption becomes the first caption.
  • fast/table/dynamic-caption-add-remove-before-child.xhtml: Added. Tests that when we remove the before child caption, the original caption becomes the first caption and is drawn.
  • fast/table/multiple-captions-crash-expected.txt: Added.
  • fast/table/multiple-captions-crash.xhtml: Added. Tests that we do not crash when table has multiple captions and captions (other than the first one) have input elements added as their child.
  • fast/table/multiple-captions-crash2-expected.txt: Added.
  • fast/table/multiple-captions-crash2.xhtml: Added. Same as multiple-captions-crash.xhtml but this testcase does not have enclosing table tags for the captions.
  • fast/table/multiple-captions-display.xhtml: Added. Tests that only the first caption and captions with position fixed (display=BLOCK) are shown. Other captions are not added to table.
  • platform/mac/fast/table/dynamic-caption-add-before-child-expected.checksum: Added.
  • platform/mac/fast/table/dynamic-caption-add-before-child-expected.png: Added.
  • platform/mac/fast/table/dynamic-caption-add-before-child-expected.txt: Added.
  • platform/mac/fast/table/dynamic-caption-add-remove-before-child-expected.checksum: Added.
  • platform/mac/fast/table/dynamic-caption-add-remove-before-child-expected.png: Added.
  • platform/mac/fast/table/dynamic-caption-add-remove-before-child-expected.txt: Added.
  • platform/mac/fast/table/multiple-captions-display-expected.checksum: Added.
  • platform/mac/fast/table/multiple-captions-display-expected.png: Added.
  • platform/mac/fast/table/multiple-captions-display-expected.txt: Added.
  • platform/mac/fast/table/prepend-in-anonymous-table-expected.png:
  • platform/mac/fast/table/prepend-in-anonymous-table-expected.txt: Rebaseline test because we do not allow captions other than the first one.
  • platform/mac/tables/mozilla_expected_failures/other/test4-expected.txt: Rebaseline test because we do not allow captions other than the first one.
Location:
trunk
Files:
16 added
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r85354 r85355  
     12011-04-21  Abhishek Arya  <inferno@chromium.org>
     2
     3        Reviewed by Dave Hyatt.
     4
     5        Tests that we do not crash on
     6        ASSERT(!m_layoutRoot->container() || !m_layoutRoot->container()->needsLayout())
     7        when a table has two or more captions.
     8        https://bugs.webkit.org/show_bug.cgi?id=58249
     9
     10        * fast/table/dynamic-caption-add-before-child.xhtml: Added. Tests that
     11        before child caption becomes the first caption.
     12        * fast/table/dynamic-caption-add-remove-before-child.xhtml: Added. Tests
     13        that when we remove the before child caption, the original caption becomes
     14        the first caption and is drawn.
     15        * fast/table/multiple-captions-crash-expected.txt: Added.
     16        * fast/table/multiple-captions-crash.xhtml: Added. Tests that we
     17        do not crash when table has multiple captions and captions (other
     18        than the first one) have input elements added as their child.
     19        * fast/table/multiple-captions-crash2-expected.txt: Added.
     20        * fast/table/multiple-captions-crash2.xhtml: Added. Same as
     21        multiple-captions-crash.xhtml but this testcase does not have
     22        enclosing table tags for the captions.
     23        * fast/table/multiple-captions-display.xhtml: Added. Tests that only
     24        the first caption and captions with position fixed (display=BLOCK) are
     25        shown. Other captions are not added to table.
     26        * platform/mac/fast/table/dynamic-caption-add-before-child-expected.checksum: Added.
     27        * platform/mac/fast/table/dynamic-caption-add-before-child-expected.png: Added.
     28        * platform/mac/fast/table/dynamic-caption-add-before-child-expected.txt: Added.
     29        * platform/mac/fast/table/dynamic-caption-add-remove-before-child-expected.checksum: Added.
     30        * platform/mac/fast/table/dynamic-caption-add-remove-before-child-expected.png: Added.
     31        * platform/mac/fast/table/dynamic-caption-add-remove-before-child-expected.txt: Added.
     32        * platform/mac/fast/table/multiple-captions-display-expected.checksum: Added.
     33        * platform/mac/fast/table/multiple-captions-display-expected.png: Added.
     34        * platform/mac/fast/table/multiple-captions-display-expected.txt: Added.
     35        * platform/mac/fast/table/prepend-in-anonymous-table-expected.png:
     36        * platform/mac/fast/table/prepend-in-anonymous-table-expected.txt: Rebaseline
     37        test because we do not allow captions other than the first one.
     38        * platform/mac/tables/mozilla_expected_failures/other/test4-expected.txt: Rebaseline
     39        test because we do not allow captions other than the first one.
     40
    1412011-04-29  Dean Jackson  <dino@apple.com>
    242
  • trunk/LayoutTests/platform/mac/fast/table/prepend-in-anonymous-table-expected.txt

    r34692 r85355  
    468468        RenderTable at (0,0) size 0x0
    469469          RenderBlock {DIV} at (0,0) size 0x0
    470           RenderBlock {DIV} at (0,0) size 0x0
    471470      RenderBlock {DIV} at (0,1474) size 769x0
    472471        RenderTable at (0,0) size 0x0
    473472          RenderBlock {DIV} at (0,0) size 0x0
    474           RenderBlock {DIV} at (0,0) size 0x0
  • trunk/LayoutTests/platform/mac/tables/mozilla_expected_failures/other/test4-expected.txt

    r78910 r85355  
    418418              RenderText {#text} at (1,1) size 47x13
    419419                text run at (1,1) width 47: "FOOTER"
    420         RenderBlock {CAPTION} at (0,0) size 0x0
    421           RenderText {#text} at (0,0) size 0x0
    422420      RenderBlock (anonymous) at (0,1204) size 769x36
    423421        RenderBR {BR} at (0,0) size 0x18
  • trunk/Source/WebCore/ChangeLog

    r85353 r85355  
     12011-04-29  Abhishek Arya  <inferno@chromium.org>
     2
     3        Reviewed by Dave Hyatt.
     4
     5        Allow only first table caption and destroy the remaining ones.
     6        https://bugs.webkit.org/show_bug.cgi?id=58249
     7
     8        Previously, we were only laying out the first table caption.
     9        However Table::layout didn't mark the other ones as not needing
     10        layout. So after table layout completes, table is marked as not
     11        needing layout with its other table caption still needing layout.
     12        This causes incorrect layout root calculations and set it to a
     13        node which is already getting deleted.
     14
     15        Tests: fast/table/dynamic-caption-add-before-child.xhtml
     16               fast/table/dynamic-caption-add-remove-before-child.xhtml
     17               fast/table/multiple-captions-crash.xhtml
     18               fast/table/multiple-captions-crash2.xhtml
     19               fast/table/multiple-captions-display.xhtml
     20
     21        * rendering/RenderTable.cpp:
     22        (WebCore::RenderTable::addChild): when new caption or a before
     23        child caption is added, we need to explicitly trigger section
     24        recalc or otherwise layout won't catch it.
     25        (WebCore::RenderTable::removeChild): when child to be removed is
     26        m_caption, make sure to trigger style recalc on the table.
     27        (WebCore::RenderTable::recalcCaption): code to destroy captions
     28        other than the first one.
     29        (WebCore::RenderTable::recalcSections): call recalcCaption
     30        helper. Store the next sibling early since child can get destroyed
     31        in recalcCaption.
     32        * rendering/RenderTable.h:
     33
    1342011-04-29  David Kilzer  <ddkilzer@apple.com>
    235
  • trunk/Source/WebCore/rendering/RenderTable.cpp

    r84815 r85355  
    117117            while (o && o != m_caption)
    118118                o = o->previousSibling();
    119             if (!o)
     119            if (!o) {
    120120                m_caption = 0;
     121                setNeedsSectionRecalc();
     122            }
    121123        }
    122124        if (!m_caption)
    123125            m_caption = toRenderBlock(child);
     126        else
     127            setNeedsSectionRecalc();
    124128        wrapInAnonymousSection = false;
    125129    } else if (child->isTableCol()) {
     
    199203{
    200204    RenderBox::removeChild(oldChild);
     205   
     206    if (m_caption && oldChild == m_caption && node())
     207        node()->setNeedsStyleRecalc();
    201208    setNeedsSectionRecalc();
    202209}
     
    689696}
    690697
     698void RenderTable::recalcCaption(RenderBlock* caption) const
     699{
     700    if (!m_caption) {
     701        m_caption = caption;
     702        m_caption->setNeedsLayout(true);
     703    } else {
     704        // Detach the child from the table.
     705        const RenderBlock* block = static_cast<const RenderBlock*>(this);
     706        const_cast<RenderBlock*>(block)->removeChild(caption);
     707
     708        // Make sure to null out the child's renderer.
     709        if (Node* node = caption->node())
     710            node->setRenderer(0);
     711
     712        // Destroy the child now.
     713        caption->destroy();
     714    }
     715}
     716
    691717void RenderTable::recalcSections() const
    692718{
     
    698724
    699725    // We need to get valid pointers to caption, head, foot and first body again
    700     for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
     726    RenderObject* nextSibling;
     727    for (RenderObject* child = firstChild(); child; child = nextSibling) {
     728        nextSibling = child->nextSibling();
    701729        switch (child->style()->display()) {
    702730            case TABLE_CAPTION:
    703                 if (!m_caption && child->isRenderBlock()) {
    704                     m_caption = toRenderBlock(child);
    705                     m_caption->setNeedsLayout(true);
    706                 }
     731                if (child->isRenderBlock())
     732                    recalcCaption(toRenderBlock(child));
    707733                break;
    708734            case TABLE_COLUMN:
  • trunk/Source/WebCore/rendering/RenderTable.h

    r81304 r85355  
    238238    void subtractCaptionRect(IntRect&) const;
    239239
     240    void recalcCaption(RenderBlock*) const;
    240241    void recalcSections() const;
    241242    void adjustLogicalHeightForCaption();
Note: See TracChangeset for help on using the changeset viewer.