Changeset 53355 in webkit


Ignore:
Timestamp:
Jan 15, 2010 6:14:25 PM (14 years ago)
Author:
eric@webkit.org
Message:

2010-01-15 Carol Szabo <​carol.szabo@nokia.com>

Reviewed by Darin Adler.

CSS2.1 Counters not updated when new elements are inserted in the DOM.
​https://bugs.webkit.org/show_bug.cgi?id=32884

  • fast/css/counters/adding-nodes-expected.txt: Added.
  • fast/css/counters/adding-nodes.html: Added.

2010-01-15 Carol Szabo <​carol.szabo@nokia.com>

Reviewed by Darin Adler.

CSS2.1 Counters not updated when new elements are inserted in the DOM.
​https://bugs.webkit.org/show_bug.cgi?id=32884

Test: fast/css/counters/adding-nodes.html

  • rendering/CounterNode.cpp: (WebCore::CounterNode::insertAfter): Modified to handle the addition of nodes with children. Needed when formerly root nodes become descendants of a new node.
  • rendering/RenderCounter.cpp: (WebCore::makeCounterNode): Changed to handle the case when root counter nodes lose their root status as a result of a new root counter node creation. (WebCore::destroyCounterNodeWithoutMapRemoval): Refactored more code into destroyCounterNodeChildren and renamed the function according to its new action. (WebCore::RenderCounter::destroyCounterNodes): Simplified to share more code with the new destroyCounterNode. (WebCore::RenderCounter::destroyCounterNode): Added to allow for selective counterNode destruction. (WebCore::RenderCounter::rendererSubtreeAttached): Added to refresh counter values in response to DOM changes. For renderers with no attached counters the execution time of this function cannot be discerned in comparison with the time needed to add a node or change the style of a node. (WebCore::updateCounters): Helper function for rendererSubtreeAttached. Updates the counters attached to a Renderer in response to the renderer or its ancestors being attached to the renderer tree.
  • rendering/RenderCounter.h:
  • rendering/RenderObject.cpp: (WebCore::RenderObject::addChild): Changed to update counter values if needed.
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r53352 r53355  
     12010-01-15  Carol Szabo  <carol.szabo@nokia.com>
     2
     3        Reviewed by Darin Adler.
     4
     5        CSS2.1 Counters not updated when new elements are inserted in the DOM.
     6        https://bugs.webkit.org/show_bug.cgi?id=32884
     7
     8        * fast/css/counters/adding-nodes-expected.txt: Added.
     9        * fast/css/counters/adding-nodes.html: Added.
     10
    1112010-01-15  Darin Fisher  <darin@chromium.org>
    212

  • trunk/WebCore/ChangeLog

    r53351 r53355  
     12010-01-15  Carol Szabo  <carol.szabo@nokia.com>
     2
     3        Reviewed by Darin Adler.
     4
     5        CSS2.1 Counters not updated when new elements are inserted in the DOM.
     6        https://bugs.webkit.org/show_bug.cgi?id=32884
     7
     8        Test: fast/css/counters/adding-nodes.html
     9
     10        * rendering/CounterNode.cpp:
     11        (WebCore::CounterNode::insertAfter):
     12        Modified to handle the addition of nodes with children. Needed when formerly
     13        root nodes become descendants of a new node.
     14        * rendering/RenderCounter.cpp:
     15        (WebCore::makeCounterNode):
     16        Changed to handle the case when root counter nodes lose their root
     17        status as a result of a new root counter node creation.
     18        (WebCore::destroyCounterNodeWithoutMapRemoval):
     19        Refactored more code into destroyCounterNodeChildren and renamed the
     20        function according to its new action.
     21        (WebCore::RenderCounter::destroyCounterNodes):
     22        Simplified to share more code with the new destroyCounterNode.
     23        (WebCore::RenderCounter::destroyCounterNode):
     24        Added to allow for selective counterNode destruction.
     25        (WebCore::RenderCounter::rendererSubtreeAttached):
     26        Added to refresh counter values in response to DOM changes.
     27        For renderers with no attached counters the execution time of this
     28        function cannot be discerned in comparison with the time needed to
     29        add a node or change the style of a node.
     30        (WebCore::updateCounters):
     31        Helper function for rendererSubtreeAttached. Updates the counters
     32        attached to a Renderer in response to the renderer or its ancestors
     33        being attached to the renderer tree.
     34        * rendering/RenderCounter.h:
     35        * rendering/RenderObject.cpp:
     36        (WebCore::RenderObject::addChild):
     37        Changed to update counter values if needed.
     38
    1392010-01-15  Alejandro G. Castro  <alex@igalia.com>
    240

  • trunk/WebCore/rendering/CounterNode.cpp

    r53230 r53355  
    2323#include "CounterNode.h"
    2424
     25#include "RenderCounter.h"
    2526#include "RenderObject.h"
    2627#include <stdio.h>
    27 
    28 // FIXME: There's currently no strategy for getting the counter tree updated when new
    29 // elements with counter-reset and counter-increment styles are added to the render tree.
    30 // Also, the code can't handle changes where an existing node needs to change into a
    31 // "reset" node, or from a "reset" node back to not a "reset" node. As of this writing,
    32 // at least some of these problems manifest as failures in the t1204-increment and
    33 // t1204-reset tests in the CSS 2.1 test suite.
    3428
    3529namespace WebCore {
    … …  
    141135    ASSERT(!refChild || refChild->m_parent == this);
    142136
     137    if (newChild->m_hasResetType) {
     138        while (m_lastChild != refChild)
     139            RenderCounter::destroyCounterNode(m_lastChild->renderer(), identifier);
     140    }
     141
    143142    CounterNode* next;
    144143
    … …  
    151150    }
    152151
    153     if (next) {
    154         ASSERT(next->m_previousSibling == refChild);
    155         next->m_previousSibling = newChild;
    156     } else {
    157         ASSERT(m_lastChild == refChild);
    158         m_lastChild = newChild;
    159     }
    160 
    161152    newChild->m_parent = this;
    162153    newChild->m_previousSibling = refChild;
    163     newChild->m_nextSibling = next;
    164 
     154
     155    if (!newChild->m_firstChild || newChild->m_hasResetType) {
     156        newChild->m_nextSibling = next;
     157        if (next) {
     158            ASSERT(next->m_previousSibling == refChild);
     159            next->m_previousSibling = newChild;
     160        } else {
     161            ASSERT(m_lastChild == refChild);
     162            m_lastChild = newChild;
     163        }
     164
     165        newChild->m_countInParent = newChild->computeCountInParent();
     166        newChild->resetRenderers(identifier);
     167        if (next)
     168            next->recount(identifier);
     169        return;
     170    }
     171
     172    // The code below handles the case when a formerly root increment counter is loosing its root position
     173    // and therefore its children become next siblings.
     174    CounterNode* last = newChild->m_lastChild;
     175    CounterNode* first = newChild->m_firstChild;
     176
     177    newChild->m_nextSibling = first;
     178    first->m_previousSibling = newChild;
     179    // The case when the original next sibling of the inserted node becomes a child of
     180    // one of the former children of the inserted node is not handled as it is believed
     181    // to be impossible since:
     182    // 1. if the increment counter node lost it's root position as a result of another
     183    //    counter node being created, it will be inserted as the last child so next is null.
     184    // 2. if the increment counter node lost it's root position as a result of a renderer being
     185    //    inserted into the document's render tree, all its former children counters are attached
     186    //    to children of the inserted renderer and hence cannot be in scope for counter nodes
     187    //    attached to renderers that were already in the document's render tree.
     188    last->m_nextSibling = next;
     189    if (next)
     190        next->m_previousSibling = last;
     191    else
     192        m_lastChild = last;
     193    for (next = first; ; next = next->m_nextSibling) {
     194        next->m_parent = this;
     195        if (last == next)
     196            break;
     197    }
     198    newChild->m_firstChild = 0;
     199    newChild->m_lastChild = 0;
    165200    newChild->m_countInParent = newChild->computeCountInParent();
    166     if (next)
    167         next->recount(identifier);
     201    newChild->resetRenderer(identifier);
     202    first->recount(identifier);
    168203}
    169204

  • trunk/WebCore/rendering/RenderCounter.cpp

    r52450 r53355  
    255255    }
    256256    nodeMap->set(identifier.impl(), newNode);
     257    if (newNode->parent() || !object->nextInPreOrder(object->parent()))
     258        return newNode;
     259    // Checking if some nodes that were previously counter tree root nodes
     260    // should become children of this node now.
     261    CounterMaps& maps = counterMaps();
     262    RenderObject* stayWithin = object->parent();
     263    for (RenderObject* currentRenderer = object->nextInPreOrder(stayWithin); currentRenderer; currentRenderer = currentRenderer->nextInPreOrder(stayWithin)) {
     264        if (!currentRenderer->m_hasCounterNodeMap)
     265            continue;
     266        CounterNode* currentCounter = maps.get(currentRenderer)->get(identifier.impl());
     267        if (!currentCounter)
     268            continue;
     269        if (currentCounter->parent()) {
     270            ASSERT(newNode->firstChild());
     271            if (currentRenderer->lastChild())
     272                currentRenderer = currentRenderer->lastChild();
     273            continue;
     274        }
     275        if (stayWithin != currentRenderer->parent() || !currentCounter->hasResetType())
     276            newNode->insertAfter(currentCounter, newNode->lastChild(), identifier);
     277        if (currentRenderer->lastChild())
     278            currentRenderer = currentRenderer->lastChild();
     279    }
    257280    return newNode;
    258281}
    … …  
    315338}
    316339
    317 static void destroyCounterNodeChildren(const AtomicString& identifier, CounterNode* node)
     340static void destroyCounterNodeWithoutMapRemoval(const AtomicString& identifier, CounterNode* node)
    318341{
    319342    CounterNode* previous;
    … …  
    330353        delete child;
    331354    }
    332 }
    333 
    334 void RenderCounter::destroyCounterNodes(RenderObject* object)
     355    RenderObject* renderer = node->renderer();
     356    if (!renderer->documentBeingDestroyed()) {
     357        if (RenderObjectChildList* children = renderer->virtualChildren())
     358            children->invalidateCounters(renderer, identifier);
     359    }
     360    if (CounterNode* parent = node->parent())
     361        parent->removeChild(node, identifier);
     362    delete node;
     363}
     364
     365void RenderCounter::destroyCounterNodes(RenderObject* renderer)
    335366{
    336367    CounterMaps& maps = counterMaps();
    337     CounterMap* map = maps.get(object);
    338     if (!map)
    339         return;
    340     maps.remove(object);
    341 
     368    CounterMaps::iterator mapsIterator = maps.find(renderer);
     369    if (mapsIterator == maps.end())
     370        return;
     371    CounterMap* map = mapsIterator->second;
    342372    CounterMap::const_iterator end = map->end();
    343373    for (CounterMap::const_iterator it = map->begin(); it != end; ++it) {
    344         CounterNode* node = it->second;
    345374        AtomicString identifier(it->first.get());
    346         destroyCounterNodeChildren(identifier, node);
    347         if (CounterNode* parent = node->parent())
    348             parent->removeChild(node, identifier);
    349         delete node;
    350     }
    351 
     375        destroyCounterNodeWithoutMapRemoval(identifier, it->second);
     376    }
     377    maps.remove(mapsIterator);
    352378    delete map;
     379    renderer->m_hasCounterNodeMap = false;
     380}
     381
     382void RenderCounter::destroyCounterNode(RenderObject* renderer, const AtomicString& identifier)
     383{
     384    CounterMap* map = counterMaps().get(renderer);
     385    if (!map)
     386        return;
     387    CounterMap::iterator mapIterator = map->find(identifier.impl());
     388    if (mapIterator == map->end())
     389        return;
     390    destroyCounterNodeWithoutMapRemoval(identifier, mapIterator->second);
     391    map->remove(mapIterator);
     392    // We do not delete "map" here even if empty because we expect to reuse
     393    // it soon. In order for a renderer to lose all its counters permanently,
     394    // a style change for the renderer involving removal of all counter
     395    // directives must occur, in which case, RenderCounter::destroyCounterNodes()
     396    // must be called.
     397    // The destruction of the Renderer (possibly caused by the removal of its
     398    // associated DOM node) is the other case that leads to the permanent
     399    // destruction of all counters attached to a Renderer. In this case
     400    // RenderCounter::destroyCounterNodes() must be and is now called, too.
     401    // RenderCounter::destroyCounterNodes() handles destruction of the counter
     402    // map associated with a renderer, so there is no risk in leaking the map.
     403}
     404
     405static void updateCounters(RenderObject* renderer)
     406{
     407    ASSERT(renderer->style());
     408    const CounterDirectiveMap* directiveMap = renderer->style()->counterDirectives();
     409    if (!directiveMap)
     410        return;
     411    CounterDirectiveMap::const_iterator end = directiveMap->end();
     412    if (!renderer->m_hasCounterNodeMap) {
     413        for (CounterDirectiveMap::const_iterator it = directiveMap->begin(); it != end; ++it)
     414            makeCounterNode(renderer, AtomicString(it->first.get()), false);
     415        return;
     416    }
     417    CounterMap* counterMap = counterMaps().get(renderer);
     418    ASSERT(counterMap);
     419    for (CounterDirectiveMap::const_iterator it = directiveMap->begin(); it != end; ++it) {
     420        CounterNode* node = counterMap->get(it->first.get());
     421        if (!node) {
     422            makeCounterNode(renderer, AtomicString(it->first.get()), false);
     423            continue;
     424        }
     425        CounterNode* newParent = 0;
     426        CounterNode* newPreviousSibling;
     427        findPlaceForCounter(renderer, AtomicString(it->first.get()), node->hasResetType(), newParent, newPreviousSibling);
     428        CounterNode* parent = node->parent();
     429        if (newParent == parent && newPreviousSibling == node->previousSibling())
     430            continue;
     431        if (parent)
     432            parent->removeChild(node, it->first.get());
     433        newParent->insertAfter(node, newPreviousSibling, it->first.get());
     434    }
     435}
     436
     437void RenderCounter::rendererSubtreeAttached(RenderObject* renderer)
     438{
     439    for (RenderObject* descendant = renderer; descendant; descendant = descendant->nextInPreOrder(renderer))
     440        updateCounters(descendant);
    353441}
    354442

  • trunk/WebCore/rendering/RenderCounter.h

    r50960 r53355  
    4141
    4242    static void destroyCounterNodes(RenderObject*);
     43    static void destroyCounterNode(RenderObject*, const AtomicString& identifier);
     44    static void rendererSubtreeAttached(RenderObject*);
    4345
    4446private:

  • trunk/WebCore/rendering/RenderObject.cpp

    r53196 r53355  
    310310        children->insertChildNode(this, newChild, beforeChild);
    311311    }
    312    
     312    RenderCounter::rendererSubtreeAttached(newChild);
    313313    if (newChild->isText() && newChild->style()->textTransform() == CAPITALIZE) {
    314314        RefPtr<StringImpl> textToTransform = toRenderText(newChild)->originalText();
Note: See TracChangeset for help on using the changeset viewer.