Changeset 125739 in webkit

Timestamp:

Aug 15, 2012 8:05:59 PM (12 years ago)

Author:

morrita@google.com

Message:

Regression(121518) TextFieldDecorationElement formatting is broken.
​https://bugs.webkit.org/show_bug.cgi?id=90913

Reviewed by Dimitri Glazkov.

Source/WebCore:

NodeRenderingContext::nextRenderer() has a problem which cannot retrieve the renderer
across an insertion point in some case. That is because ad-hoc composed tree traversal on
NodeRenderingContext is broken. The problem is hidden before r121518 though.

This change rewrite nextRenderer() using ComposedShadowTreeWalker to eliminate the ad-hoc
traversal. previousRenderer() is also rewritten in the same way.

Test: fast/dom/shadow/shadow-div-reflow.html

• dom/NodeRenderingContext.cpp:

(WebCore):
(WebCore::NodeRenderingContext::nextRenderer):
(WebCore::NodeRenderingContext::previousRenderer):

LayoutTests:

• fast/dom/shadow/shadow-div-reflow-expected.html: Added.
• fast/dom/shadow/shadow-div-reflow.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/shadow/shadow-div-reflow-expected.html (added)
• LayoutTests/fast/dom/shadow/shadow-div-reflow.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/NodeRenderingContext.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-08-15  MORITA Hajime  <morrita@google.com>
+
+        Regression(121518) TextFieldDecorationElement formatting is broken.
+        https://bugs.webkit.org/show_bug.cgi?id=90913
+
+        Reviewed by Dimitri Glazkov.
+
+        * fast/dom/shadow/shadow-div-reflow-expected.html: Added.
+        * fast/dom/shadow/shadow-div-reflow.html: Added.
+
 2012-08-14  Jeffrey Pfau  <jpfau@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-08-15  MORITA Hajime  <morrita@google.com>
+
+        Regression(121518) TextFieldDecorationElement formatting is broken.
+        https://bugs.webkit.org/show_bug.cgi?id=90913
+
+        Reviewed by Dimitri Glazkov.
+
+        NodeRenderingContext::nextRenderer() has a problem which cannot retrieve the renderer
+        across an insertion point in some case. That is because ad-hoc composed tree traversal on
+        NodeRenderingContext is broken. The problem is hidden before r121518 though.
+
+        This change rewrite nextRenderer() using ComposedShadowTreeWalker to eliminate the ad-hoc
+        traversal. previousRenderer() is also rewritten in the same way.
+
+        Test: fast/dom/shadow/shadow-div-reflow.html
+
+        * dom/NodeRenderingContext.cpp:
+        (WebCore):
+        (WebCore::NodeRenderingContext::nextRenderer):
+        (WebCore::NodeRenderingContext::previousRenderer):
+
 2012-08-15  Julien Chaffraix  <jchaffraix@webkit.org>
 

trunk/Source/WebCore/dom/NodeRenderingContext.cpp

@@ -50 +50 @@
 using namespace HTMLNames;
 
-static RenderObject* firstRendererOf(Node*);
-static RenderObject* lastRendererOf(Node*);
-
 NodeRenderingContext::NodeRenderingContext(Node* node)
     : m_node(node)
@@ -83 +80 @@
 }
 
-static inline RenderObject* nextRendererOfInsertionPoint(InsertionPoint* parent, Node* current)
-{
-    size_t start = parent->indexOf(current);
-    if (notFound == start)
-        return 0;
-
-    for (size_t i = start + 1; i < parent->size(); ++i) {
-        if (RenderObject* renderer = parent->at(i)->renderer())
-            return renderer;
-    }
-
-    return 0;
-}
-
-static inline RenderObject* previousRendererOfInsertionPoint(InsertionPoint* parent, Node* current)
-{
-    RenderObject* lastRenderer = 0;
-
-    for (size_t i = 0; i < parent->size(); ++i) {
-        if (parent->at(i) == current)
-            break;
-        if (RenderObject* renderer = parent->at(i)->renderer())
-            lastRenderer = renderer;
-    }
-
-    return lastRenderer;
-}
-
-static inline RenderObject* firstRendererOfInsertionPoint(InsertionPoint* parent)
-{
-    size_t size = parent->size();
-    for (size_t i = 0; i < size; ++i) {
-        if (RenderObject* renderer = parent->at(i)->renderer())
-            return renderer;
-    }
-
-    return firstRendererOf(parent->firstChild());
-}
-
-static inline RenderObject* lastRendererOfInsertionPoint(InsertionPoint* parent)
-{
-    size_t size = parent->size();
-    for (size_t i = 0; i < size; ++i) {
-        if (RenderObject* renderer = parent->at(size - 1 - i)->renderer())
-            return renderer;
-    }
-
-    return lastRendererOf(parent->lastChild());
-}
-
-static inline RenderObject* firstRendererOf(Node* node)
-{
-    for (; node; node = node->nextSibling()) {
-        if (node->renderer()) {
-            // Do not return elements that are attached to a different flow-thread.
-            if (node->renderer()->style() && !node->renderer()->style()->flowThread().isEmpty())
-                continue;
-            return node->renderer();
-        }
-
-        if (isInsertionPoint(node) && toInsertionPoint(node)->isActive()) {
-            if (RenderObject* first = firstRendererOfInsertionPoint(toInsertionPoint(node)))
-                return first;
-        }
-    }
-
-    return 0;
-}
-
-static inline RenderObject* lastRendererOf(Node* node)
-{
-    for (; node; node = node->previousSibling()) {
-        if (node->renderer()) {
-            // Do not return elements that are attached to a different flow-thread.
-            if (node->renderer()->style() && !node->renderer()->style()->flowThread().isEmpty())
-                continue;
-            return node->renderer();
-        }
-        if (isInsertionPoint(node) && toInsertionPoint(node)->isActive()) {
-            if (RenderObject* last = lastRendererOfInsertionPoint(toInsertionPoint(node)))
-                return last;
-        }
-    }
-
-    return 0;
-}
-
 RenderObject* NodeRenderingContext::nextRenderer() const
 {
@@ -178 +88 @@
         return m_parentFlowRenderer->nextRendererForNode(m_node);
 
-    if (m_parentDetails.insertionPoint()) {
-        if (RenderObject* found = nextRendererOfInsertionPoint(m_parentDetails.insertionPoint(), m_node))
-            return found;
-        return NodeRenderingContext(m_parentDetails.insertionPoint()).nextRenderer();
-    }
-
     // Avoid an O(N^2) problem with this function by not checking for
     // nextRenderer() when the parent element hasn't attached yet.
-    if (m_node->parentOrHostNode() && !m_node->parentOrHostNode()->attached())
+    if (m_parentDetails.node() && !m_parentDetails.node()->attached())
         return 0;
 
-    return firstRendererOf(m_node->nextSibling());
+    ComposedShadowTreeWalker walker(m_node);
+    do {
+        walker.nextSibling();
+        if (!walker.get())
+            return 0;
+        if (RenderObject* renderer = walker.get()->renderer()) {
+            // Do not return elements that are attached to a different flow-thread.
+            if (renderer->style() && !renderer->style()->flowThread().isEmpty())
+                continue;
+            return renderer;
+        }
+    } while (true);
+
+    ASSERT_NOT_REACHED();
+    return 0;
 }
 
@@ -200 +118 @@
         return m_parentFlowRenderer->previousRendererForNode(m_node);
 
-    if (m_parentDetails.insertionPoint()) {
-        if (RenderObject* found = previousRendererOfInsertionPoint(m_parentDetails.insertionPoint(), m_node))
-            return found;
-        return NodeRenderingContext(m_parentDetails.insertionPoint()).previousRenderer();
-    }
-
     // FIXME: We should have the same O(N^2) avoidance as nextRenderer does
     // however, when I tried adding it, several tests failed.
-    return lastRendererOf(m_node->previousSibling());
+
+    ComposedShadowTreeWalker walker(m_node);
+    do {
+        walker.previousSibling();
+        if (!walker.get())
+            return 0;
+        if (RenderObject* renderer = walker.get()->renderer()) {
+            // Do not return elements that are attached to a different flow-thread.
+            if (renderer->style() && !renderer->style()->flowThread().isEmpty())
+                continue;
+            return renderer;
+        }
+    } while (true);
+
+    ASSERT_NOT_REACHED();
+    return 0;
 }