Changeset 58665 in webkit

Timestamp:

May 2, 2010 9:25:27 PM (14 years ago)

Author:

mitz@apple.com

Message:

Another case of <rdar://problem/7552959> REGRESSION: Infinite recursion in Position::getInlineBoxAndOffset()
​https://bugs.webkit.org/show_bug.cgi?id=38445

Reviewed by Simon Fraser.

WebCore:

Test: editing/selection/mixed-editability-11.html

• dom/Position.cpp:

(WebCore::downstreamIgnoringEditingBoundaries): Added. Returns the furthest visually equivalent
position downstream, crossing any editability boundaries.
(WebCore::upstreamIgnoringEditingBoundaries): Similarly for upstream.
(WebCore::Position::getInlineBoxAndOffset): Changed the logic for finding an inline box for positions
whose node is a block flow. Instead of traversing the DOM, advance downstream or upstream as far as
possible, crossing any editability boudaries. Infinite recursion is avoided by advancing all the way
and checking that the new position is different from the starting position. Also replaced the specific
test for buttons with the generic and more comprehensive canHaveChildrenForEditing().

LayoutTests:

• editing/selection/mixed-editability-11-expected.txt: Added.
• editing/selection/mixed-editability-11.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/selection/mixed-editability-11-expected.txt (added)
• LayoutTests/editing/selection/mixed-editability-11.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/dom/Position.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2010-05-02  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Simon Fraser.
+
+        Another case of <rdar://problem/7552959> REGRESSION: Infinite recursion in Position::getInlineBoxAndOffset()
+        https://bugs.webkit.org/show_bug.cgi?id=38445
+
+        * editing/selection/mixed-editability-11-expected.txt: Added.
+        * editing/selection/mixed-editability-11.html: Added.
+
 2010-05-02  Dirk Schulze  <krit@webkit.org>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-05-02  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Simon Fraser.
+
+        Another case of <rdar://problem/7552959> REGRESSION: Infinite recursion in Position::getInlineBoxAndOffset()
+        https://bugs.webkit.org/show_bug.cgi?id=38445
+
+        Test: editing/selection/mixed-editability-11.html
+
+        * dom/Position.cpp:
+        (WebCore::downstreamIgnoringEditingBoundaries): Added. Returns the furthest visually equivalent
+        position downstream, crossing any editability boundaries.
+        (WebCore::upstreamIgnoringEditingBoundaries): Similarly for upstream.
+        (WebCore::Position::getInlineBoxAndOffset): Changed the logic for finding an inline box for positions
+        whose node is a block flow. Instead of traversing the DOM, advance downstream or upstream as far as
+        possible, crossing any editability boudaries. Infinite recursion is avoided by advancing all the way
+        and checking that the new position is different from the starting position. Also replaced the specific
+        test for buttons with the generic and more comprehensive canHaveChildrenForEditing().
+
 2010-05-02  Tasuku Suzuki  <tasuku.suzuki@nokia.com>
 

trunk/WebCore/dom/Position.cpp

@@ -987 +987 @@
 }
 
+static Position downstreamIgnoringEditingBoundaries(Position position)
+{
+    Position lastPosition;
+    while (position != lastPosition) {
+        lastPosition = position;
+        position = position.downstream(Position::CanCrossEditingBoundary);
+    }
+    return position;
+}
+
+static Position upstreamIgnoringEditingBoundaries(Position position)
+{
+    Position lastPosition;
+    while (position != lastPosition) {
+        lastPosition = position;
+        position = position.upstream(Position::CanCrossEditingBoundary);
+    }
+    return position;
+}
+
 void Position::getInlineBoxAndOffset(EAffinity affinity, TextDirection primaryDirection, InlineBox*& inlineBox, int& caretOffset) const
 {
     caretOffset = m_offset;
     RenderObject* renderer = node()->renderer();
-          
+
     if (!renderer->isText()) {
-        if (!renderer->isRenderButton() && renderer->isBlockFlow() && hasRenderedNonAnonymousDescendantsWithHeight(renderer)) {
-            bool lastPosition = caretOffset == lastOffsetInNode(node());
-            Node* startNode = lastPosition ? node()->childNode(caretOffset - 1) : node()->childNode(caretOffset);
-            while (startNode && (!startNode->renderer() || (startNode->isTextNode() && toRenderText(startNode->renderer())->isAllCollapsibleWhitespace())))
-                startNode = (lastPosition)? startNode->previousSibling(): startNode->nextSibling();
-            if (startNode) {
-                Position pos(startNode, 0);
-                pos = pos.downstream(CanCrossEditingBoundary);
-                pos.getInlineBoxAndOffset(UPSTREAM, primaryDirection, inlineBox, caretOffset);
-                if (lastPosition && inlineBox)
-                    caretOffset = inlineBox->caretMaxOffset();
+        inlineBox = 0;
+        if (canHaveChildrenForEditing(node()) && renderer->isBlockFlow() && hasRenderedNonAnonymousDescendantsWithHeight(renderer)) {
+            // Try a visually equivalent position with possibly opposite editability. This helps in case |this| is in
+            // an editable block but surrounded by non-editable positions. It acts to negate the logic at the beginning
+            // of RenderObject::createVisiblePosition().
+            Position equivalent = downstreamIgnoringEditingBoundaries(*this);
+            if (equivalent == *this)
+                equivalent = upstreamIgnoringEditingBoundaries(*this);
+            if (equivalent == *this)
                 return;
-            }
-        }
-        inlineBox = 0;
+
+            equivalent.getInlineBoxAndOffset(UPSTREAM, primaryDirection, inlineBox, caretOffset);
+            return;
+        }
         if (renderer->isBox()) {
             inlineBox = toRenderBox(renderer)->inlineBoxWrapper();