Changeset 53868 in webkit


Ignore:
Timestamp:
Jan 26, 2010 2:48:56 PM (14 years ago)
Author:
mitz@apple.com
Message:

<rdar://problem/7576663> Crash caused by anonymous list item
https://bugs.webkit.org/show_bug.cgi?id=34183

Reviewed by Beth Dakin.

WebCore:

Test: fast/lists/anonymous-items.html

enclosingList() and previousListItem() were DOM-based, but in order to work with anonymous
list items, they need to work with rthe render tree.

  • rendering/RenderListItem.cpp:

(WebCore::isList): Factored out.
(WebCore::enclosingList): Added this variant that takes a RenderObject.
(WebCore::previousListItem): Changed to travers the render tree.
(WebCore::RenderListItem::calcValue): Use the RenderObject version of enclosingList()
(WebCore::RenderListItem::setExplicitValue): Added an assertion.
(WebCore::RenderListItem::clearExplicitValue): Ditto.

LayoutTests:

  • fast/lists/anonymous-items.html: Added.
  • platform/mac/fast/lists/anonymous-items-expected.checksum: Added.
  • platform/mac/fast/lists/anonymous-items-expected.png: Added.
  • platform/mac/fast/lists/anonymous-items-expected.txt: Added.
Location:
trunk
Files:
4 added
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r53863 r53868  
     12010-01-26  Dan Bernstein  <mitz@apple.com>
     2
     3        Reviewed by Beth Dakin.
     4
     5        <rdar://problem/7576663> Crash caused by anonymous list item
     6        https://bugs.webkit.org/show_bug.cgi?id=34183
     7
     8        * fast/lists/anonymous-items.html: Added.
     9        * platform/mac/fast/lists/anonymous-items-expected.checksum: Added.
     10        * platform/mac/fast/lists/anonymous-items-expected.png: Added.
     11        * platform/mac/fast/lists/anonymous-items-expected.txt: Added.
     12
    1132010-01-26  Chris Fleizach  <cfleizach@apple.com>
    214
  • trunk/WebCore/ChangeLog

    r53867 r53868  
     12010-01-26  Dan Bernstein  <mitz@apple.com>
     2
     3        Reviewed by Beth Dakin.
     4
     5        <rdar://problem/7576663> Crash caused by anonymous list item
     6        https://bugs.webkit.org/show_bug.cgi?id=34183
     7
     8        Test: fast/lists/anonymous-items.html
     9
     10        enclosingList() and previousListItem() were DOM-based, but in order to work with anonymous
     11        list items, they need to work with rthe render tree.
     12
     13        * rendering/RenderListItem.cpp:
     14        (WebCore::isList): Factored out.
     15        (WebCore::enclosingList): Added this variant that takes a RenderObject.
     16        (WebCore::previousListItem): Changed to travers the render tree.
     17        (WebCore::RenderListItem::calcValue): Use the RenderObject version of enclosingList()
     18        (WebCore::RenderListItem::setExplicitValue): Added an assertion.
     19        (WebCore::RenderListItem::clearExplicitValue): Ditto.
     20
    1212010-01-26  Brian Weinstein  <bweinstein@apple.com>
    222
  • trunk/WebCore/rendering/RenderListItem.cpp

    r52521 r53868  
    22 * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
    33 *           (C) 1999 Antti Koivisto (koivisto@kde.org)
    4  * Copyright (C) 2003, 2004, 2005, 2006 Apple Computer, Inc.
     4 * Copyright (C) 2003, 2004, 2005, 2006, 2010 Apple Inc. All rights reserved.
    55 * Copyright (C) 2006 Andrew Wellington (proton@wiretapped.net)
    66 *
     
    7676}
    7777
     78static bool isList(Node* node)
     79{
     80    return (node->hasTagName(ulTag) || node->hasTagName(olTag));
     81}
     82
    7883static Node* enclosingList(Node* node)
    7984{
    8085    Node* parent = node->parentNode();
    8186    for (Node* n = parent; n; n = n->parentNode())
    82         if (n->hasTagName(ulTag) || n->hasTagName(olTag))
     87        if (isList(n))
    8388            return n;
    8489    // If there's no actual <ul> or <ol> list element, then our parent acts as
     
    8893}
    8994
     95static Node* enclosingList(const RenderObject* renderer)
     96{
     97    Node* node = renderer->node();
     98    if (node)
     99        return enclosingList(node);
     100
     101    renderer = renderer->parent();
     102    while (renderer && !renderer->node())
     103        renderer = renderer->parent();
     104
     105    node = renderer->node();
     106    if (isList(node))
     107        return node;
     108
     109    return enclosingList(node);
     110}
     111
    90112static RenderListItem* previousListItem(Node* list, const RenderListItem* item)
    91113{
    92     for (Node* node = item->node()->traversePreviousNode(); node != list; node = node->traversePreviousNode()) {
    93         RenderObject* renderer = node->renderer();
    94         if (!renderer || !renderer->isListItem())
     114    for (RenderObject* renderer = item->previousInPreOrder(); renderer != list->renderer(); renderer = renderer->previousInPreOrder()) {
     115        if (!renderer->isListItem())
    95116            continue;
    96         Node* otherList = enclosingList(node);
     117        Node* otherList = enclosingList(renderer);
    97118        // This item is part of our current list, so it's what we're looking for.
    98119        if (list == otherList)
    99120            return toRenderListItem(renderer);
    100121        // We found ourself inside another list; lets skip the rest of it.
    101         // Use traverseNextNode() here because the other list itself may actually
     122        // Use nextInPreOrder() here because the other list itself may actually
    102123        // be a list item itself. We need to examine it, so we do this to counteract
    103         // the traversePreviousNode() that will be done by the loop.
     124        // the previousInPreOrder() that will be done by the loop.
    104125        if (otherList)
    105             node = otherList->traverseNextNode();
     126            renderer = otherList->renderer()->nextInPreOrder();
    106127    }
    107128    return 0;
     
    112133    if (m_hasExplicitValue)
    113134        return m_explicitValue;
    114     Node* list = enclosingList(node());
     135    Node* list = enclosingList(this);
    115136    // FIXME: This recurses to a possible depth of the length of the list.
    116137    // That's not good -- we need to change this to an iterative algorithm.
     
    323344void RenderListItem::setExplicitValue(int value)
    324345{
     346    ASSERT(node());
     347
    325348    if (m_hasExplicitValue && m_explicitValue == value)
    326349        return;
     
    333356void RenderListItem::clearExplicitValue()
    334357{
     358    ASSERT(node());
     359
    335360    if (!m_hasExplicitValue)
    336361        return;
Note: See TracChangeset for help on using the changeset viewer.