Changeset 56989 in webkit

Timestamp:

Apr 2, 2010 1:31:14 AM (14 years ago)

Author:

eric@webkit.org

Message:

2010-04-02 Roland Steiner <​rolandsteiner@chromium.org>

Reviewed by Eric Seidel.

Bug 36741 - Duplicate, slightly divergent implementation of Position[Iterator]::isCandidate()
​https://bugs.webkit.org/show_bug.cgi?id=36741

Patch: change Position::isCandididate() to call the PositionIterator::isCandidate() version.
Update PositionIterator::isCandidate() to mirror Position::isCandidate().

Rationale: PositionIterator::isCandidate() is called in a tight loop within
next/previousCandidate(). Also, creation of a PositionIterator from a Position
is cheaper than vice-versa.

Tests: ran all tests in 'editing'.

• dom/Position.cpp: (WebCore::Position::isCandidate):
• dom/PositionIterator.cpp: (WebCore::PositionIterator::isCandidate):

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Position.cpp (modified) (1 diff)
• dom/PositionIterator.cpp (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-04-02  Roland Steiner  <rolandsteiner@chromium.org>
+
+        Reviewed by Eric Seidel.
+
+        Bug 36741 -  Duplicate, slightly divergent implementation of Position[Iterator]::isCandidate()
+        https://bugs.webkit.org/show_bug.cgi?id=36741
+        
+        Patch: change Position::isCandididate() to call the PositionIterator::isCandidate() version.
+        Update PositionIterator::isCandidate() to mirror Position::isCandidate().
+
+        Rationale: PositionIterator::isCandidate() is called in a tight loop within
+        next/previousCandidate(). Also, creation of a PositionIterator from a Position
+        is cheaper than vice-versa.
+
+        Tests: ran all tests in 'editing'.
+
+        * dom/Position.cpp:
+        (WebCore::Position::isCandidate):
+        * dom/PositionIterator.cpp:
+        (WebCore::PositionIterator::isCandidate):
+
 2010-04-02  Steve Falkenburg  <sfalken@apple.com>
 

trunk/WebCore/dom/Position.cpp

@@ -726 +726 @@
 bool Position::isCandidate() const
 {
-    if (isNull())
-        return false;
-        
-    RenderObject *renderer = node()->renderer();
-    if (!renderer)
-        return false;
-    
-    if (renderer->style()->visibility() != VISIBLE)
-        return false;
-
-    if (renderer->isBR())
-        return m_offset == 0 && !nodeIsUserSelectNone(node()->parent());
-
-    if (renderer->isText())
-        return inRenderedText() && !nodeIsUserSelectNone(node());
-
-    if (isTableElement(node()) || editingIgnoresContent(node()))
-        return (atFirstEditingPositionForNode() || atLastEditingPositionForNode()) && !nodeIsUserSelectNone(node()->parent());
-
-    if (m_anchorNode->hasTagName(htmlTag))
-        return false;
-        
-    if (renderer->isBlockFlow()) {
-        if (toRenderBlock(renderer)->height() || m_anchorNode->hasTagName(bodyTag)) {
-            if (!Position::hasRenderedNonAnonymousDescendantsWithHeight(renderer))
-                return atFirstEditingPositionForNode() && !Position::nodeIsUserSelectNone(node());
-            return m_anchorNode->isContentEditable() && !Position::nodeIsUserSelectNone(node()) && atEditingBoundary();
-        }
-    } else
-        return m_anchorNode->isContentEditable() && !Position::nodeIsUserSelectNone(node()) && atEditingBoundary();
-
-    return false;
+    return PositionIterator(*this).isCandidate();
 }
 

trunk/WebCore/dom/PositionIterator.cpp

@@ -163 +163 @@
             return m_anchorNode->isContentEditable() && !Position::nodeIsUserSelectNone(m_anchorNode) && Position(*this).atEditingBoundary();
         }
-    }
+    } else 
+        return m_anchorNode->isContentEditable() && !Position::nodeIsUserSelectNone(m_anchorNode) && Position(*this).atEditingBoundary();
 
     return false;