Changeset 136107 in webkit


Ignore:
Timestamp:
Nov 29, 2012 12:35:50 AM (11 years ago)
Author:
commit-queue@webkit.org
Message:

[CSS Regions] Fix content node renderers ordering inside the named flow thread
​https://bugs.webkit.org/show_bug.cgi?id=103501

Patch by Andrei Bucur <​abucur@adobe.com> on 2012-11-29
Reviewed by David Hyatt.

Source/WebCore:

This patch fixes two issues with how content nodes renderers are added to a named flow thread.
The first issue was about determining the insertion position of a renderer inside the children list of a named flow thread. Before this patch, the
insertion point was based on both the DOM ordering of the elements and insertion order of previous renderers.
The patch fixes this and makes the renderer position just a function of the DOM ordering of elements.
The second issue appeared when next/previousRenderer methods were skipping nodes because they had the flow-into property as a side effect
of copying the style of the parent element (e.g. Text nodes). The patch ensures the skipped nodes are also elements.

Tests: fast/regions/named-flow-content-order-1.html

fast/regions/named-flow-content-order-2.html
fast/regions/named-flow-content-order-3.html

  • dom/NodeRenderingContext.cpp:

(WebCore::NodeRenderingContext::nextRenderer): Skip only elements.
(WebCore::NodeRenderingContext::previousRenderer): Skip only elements.

  • rendering/RenderNamedFlowThread.cpp:

(WebCore::RenderNamedFlowThread::addFlowChild): Insert the renderer in the list based on the DOM position of the owner element.

  • rendering/RenderNamedFlowThread.h:

(RenderNamedFlowThread):

  • rendering/RenderObject.cpp:

(WebCore::RenderObject::renderNamedFlowThreadWrapper): Rename to eliminate the confusion with enclosingRenderFlowThread.
(WebCore::RenderObject::insertedIntoTree):
(WebCore::RenderObject::willBeRemovedFromTree):

  • rendering/RenderObject.h:

(RenderObject):

LayoutTests:

The first two ref tests cover the issue with incorrectly computing the insertion position for a content node renderer.
The third ref test covers the issue with nextRenderer and previousRenderer skipping valid nodes.

  • fast/regions/named-flow-content-order-1-expected.html: Added.
  • fast/regions/named-flow-content-order-1.html: Added.
  • fast/regions/named-flow-content-order-2-expected.html: Added.
  • fast/regions/named-flow-content-order-2.html: Added.
  • fast/regions/named-flow-content-order-3-expected.html: Added.
  • fast/regions/named-flow-content-order-3.html: Added.
Location:
trunk
Files:
6 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r136105 r136107  
     12012-11-29  Andrei Bucur  <abucur@adobe.com>
     2
     3        [CSS Regions] Fix content node renderers ordering inside the named flow thread
     4        https://bugs.webkit.org/show_bug.cgi?id=103501
     5
     6        Reviewed by David Hyatt.
     7
     8        The first two ref tests cover the issue with incorrectly computing the insertion position for a content node renderer.
     9        The third ref test covers the issue with nextRenderer and previousRenderer skipping valid nodes.
     10
     11        * fast/regions/named-flow-content-order-1-expected.html: Added.
     12        * fast/regions/named-flow-content-order-1.html: Added.
     13        * fast/regions/named-flow-content-order-2-expected.html: Added.
     14        * fast/regions/named-flow-content-order-2.html: Added.
     15        * fast/regions/named-flow-content-order-3-expected.html: Added.
     16        * fast/regions/named-flow-content-order-3.html: Added.
     17
    1182012-11-28  Mike West  <mkwst@chromium.org>
    219

  • trunk/Source/WebCore/ChangeLog

    r136105 r136107  
     12012-11-29  Andrei Bucur  <abucur@adobe.com>
     2
     3        [CSS Regions] Fix content node renderers ordering inside the named flow thread
     4        https://bugs.webkit.org/show_bug.cgi?id=103501
     5
     6        Reviewed by David Hyatt.
     7
     8        This patch fixes two issues with how content nodes renderers are added to a named flow thread.
     9        The first issue was about determining the insertion position of a renderer inside the children list of a named flow thread. Before this patch, the
     10        insertion point was based on both the DOM ordering of the elements and insertion order of previous renderers.
     11        The patch fixes this and makes the renderer position just a function of the DOM ordering of elements.
     12        The second issue appeared when next/previousRenderer methods were skipping nodes because they had the flow-into property as a side effect
     13        of copying the style of the parent element (e.g. Text nodes). The patch ensures the skipped nodes are also elements.
     14
     15        Tests: fast/regions/named-flow-content-order-1.html
     16               fast/regions/named-flow-content-order-2.html
     17               fast/regions/named-flow-content-order-3.html
     18
     19        * dom/NodeRenderingContext.cpp:
     20        (WebCore::NodeRenderingContext::nextRenderer): Skip only elements.
     21        (WebCore::NodeRenderingContext::previousRenderer): Skip only elements.
     22        * rendering/RenderNamedFlowThread.cpp:
     23        (WebCore::RenderNamedFlowThread::addFlowChild): Insert the renderer in the list based on the DOM position of the owner element.
     24        * rendering/RenderNamedFlowThread.h:
     25        (RenderNamedFlowThread):
     26        * rendering/RenderObject.cpp:
     27        (WebCore::RenderObject::renderNamedFlowThreadWrapper): Rename to eliminate the confusion with enclosingRenderFlowThread.
     28        (WebCore::RenderObject::insertedIntoTree):
     29        (WebCore::RenderObject::willBeRemovedFromTree):
     30        * rendering/RenderObject.h:
     31        (RenderObject):
     32
    1332012-11-28  Mike West  <mkwst@chromium.org>
    234

  • trunk/Source/WebCore/dom/NodeRenderingContext.cpp

    r136037 r136107  
    8787    for (walker.nextSibling(); walker.get(); walker.nextSibling()) {
    8888        if (RenderObject* renderer = walker.get()->renderer()) {
    89             // Do not return elements that are attached to a different flow-thread.
    90             if (renderer->style() && !renderer->style()->flowThread().isEmpty())
     89            // Renderers for elements attached to a flow thread should be skipped because they are parented differently.
     90            if (renderer->node()->isElementNode() && renderer->style() && !renderer->style()->flowThread().isEmpty())
    9191                continue;
    9292            return renderer;
    … …  
    111111    for (walker.previousSibling(); walker.get(); walker.previousSibling()) {
    112112        if (RenderObject* renderer = walker.get()->renderer()) {
    113             // Do not return elements that are attached to a different flow-thread.
    114             if (renderer->style() && !renderer->style()->flowThread().isEmpty())
     113            // Renderers for elements attached to a flow thread should be skipped because they are parented differently.
     114            if (renderer->node()->isElementNode() && renderer->style() && !renderer->style()->flowThread().isEmpty())
    115115                continue;
    116116            return renderer;

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

    r133779 r136107  
    114114}
    115115
    116 void RenderNamedFlowThread::addFlowChild(RenderObject* newChild, RenderObject* beforeChild)
     116void RenderNamedFlowThread::addFlowChild(RenderObject* newChild)
    117117{
    118118    // The child list is used to sort the flow thread's children render objects
    119119    // based on their corresponding nodes DOM order. The list is needed to avoid searching the whole DOM.
    120120
     121    Node* childNode = newChild->node();
     122
    121123    // Do not add anonymous objects.
    122     if (!newChild->node())
     124    if (!childNode)
    123125        return;
    124126
     127    ASSERT(childNode->isElementNode());
     128
     129    RenderObject* beforeChild = nextRendererForNode(childNode);
    125130    if (beforeChild)
    126131        m_flowThreadChildList.insertBefore(beforeChild, newChild);

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

    r130918 r136107  
    5656    RenderObject* previousRendererForNode(Node*) const;
    5757
    58     void addFlowChild(RenderObject* newChild, RenderObject* beforeChild = 0);
     58    void addFlowChild(RenderObject* newChild);
    5959    void removeFlowChild(RenderObject*);
    6060    bool hasChildren() const { return !m_flowThreadChildList.isEmpty(); }

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

    r136054 r136107  
    605605}
    606606
    607 RenderNamedFlowThread* RenderObject::enclosingRenderNamedFlowThread() const
     607RenderNamedFlowThread* RenderObject::renderNamedFlowThreadWrapper() const
    608608{
    609609    RenderObject* object = const_cast<RenderObject*>(this);
    … …  
    24232423        parent()->dirtyLinesFromChangedChild(this);
    24242424
    2425     if (RenderNamedFlowThread* containerFlowThread = parent()->enclosingRenderNamedFlowThread())
     2425    if (RenderNamedFlowThread* containerFlowThread = parent()->renderNamedFlowThreadWrapper())
    24262426        containerFlowThread->addFlowChild(this);
    24272427}
    … …  
    24512451        removeFromRenderFlowThread();
    24522452
    2453     if (RenderNamedFlowThread* containerFlowThread = parent()->enclosingRenderNamedFlowThread())
     2453    if (RenderNamedFlowThread* containerFlowThread = parent()->renderNamedFlowThreadWrapper())
    24542454        containerFlowThread->removeFlowChild(this);
    24552455

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

    r136045 r136107  
    242242    RenderFlowThread* enclosingRenderFlowThread() const;
    243243
    244     RenderNamedFlowThread* enclosingRenderNamedFlowThread() const;
     244    RenderNamedFlowThread* renderNamedFlowThreadWrapper() const;
    245245
    246246    virtual bool isEmpty() const { return firstChild() == 0; }
Note: See TracChangeset for help on using the changeset viewer.