Reviewed by Maciej, Darin.
authorharrison <harrison@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 May 2005 00:45:31 +0000 (00:45 +0000)
committerharrison <harrison@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 May 2005 00:45:31 +0000 (00:45 +0000)
        <rdar://problem/4103339> VisiblePosition and PositionIterator iterators do not return positions in order

        * WebCore.pbproj/project.pbxproj:
        Removed dom_positioniterator.h and dom_positioniterator.cpp.

        * khtml/editing/htmlediting.cpp:
        Removed unused include of dom_positioniterator.h and "using" of PositionIterator.

        * khtml/editing/selection.cpp:
        Removed unused include of dom_positioniterator.h.

        * khtml/editing/visible_position.h:
        * khtml/editing/visible_position.cpp:
        (khtml::VisiblePosition::previousVisiblePosition):
        (khtml::VisiblePosition::nextVisiblePosition):
        (khtml::VisiblePosition::downstreamDeepEquivalent):
        Use Position::next(), Position::previous(), Position::atStart(), Position::atEnd() instead of duplicated code.

        * khtml/xml/dom_nodeimpl.h:
        * khtml/xml/dom_nodeimpl.cpp:
        (NodeImpl::maxDeepOffset):
        Added to support Position::next(), Position::previous(), Position::atStart(), Position::atEnd()

        * khtml/xml/dom_position.h:
        * khtml/xml/dom_position.cpp:
        (DOM::Position::previous):
        (DOM::Position::next):
        (DOM::Position::atStart):
        (DOM::Position::atEnd):
        Moved here, replacing VisiblePosition's duplicate and PositionIterator.  Fixed to
        return positions in order and not skip positions.

        (DOM::Position::previousCharacterPosition):
        (DOM::Position::nextCharacterPosition):
        Use Position::next(), Position::previous(), Position::atStart(), Position::atEnd() instead of PositionIterator.

        (DOM::isStreamer):
        (DOM::Position::upstream):
        (DOM::Position::downstream):
        Use Position::next(), Position::previous(), Position::atStart(), Position::atEnd() instead of PositionIterator.

        * khtml/xml/dom_positioniterator.cpp: Removed.
        * khtml/xml/dom_positioniterator.h: Removed.
        Removed in favor of Position::next(), Position::previous(), Position::atStart(), Position::atEnd()

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@9130 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
WebCore/ChangeLog-2005-08-23
WebCore/WebCore.pbproj/project.pbxproj
WebCore/khtml/editing/SelectionController.cpp
WebCore/khtml/editing/htmlediting.cpp
WebCore/khtml/editing/selection.cpp
WebCore/khtml/editing/visible_position.cpp
WebCore/khtml/editing/visible_position.h
WebCore/khtml/xml/dom_nodeimpl.cpp
WebCore/khtml/xml/dom_nodeimpl.h
WebCore/khtml/xml/dom_position.cpp
WebCore/khtml/xml/dom_position.h
WebCore/khtml/xml/dom_positioniterator.cpp [deleted file]
WebCore/khtml/xml/dom_positioniterator.h [deleted file]

index be3fe2631a3036554b8acfd1b2724d3dd0564e26..ffd2d1e52932fa0aebac565a4f9e5b5d2edeea12 100644 (file)
@@ -1,3 +1,52 @@
+2005-05-06  David Harrison  <harrison@apple.com>
+
+        Reviewed by Maciej, Darin.
+
+        <rdar://problem/4103339> VisiblePosition and PositionIterator iterators do not return positions in order
+
+        * WebCore.pbproj/project.pbxproj:
+        Removed dom_positioniterator.h and dom_positioniterator.cpp.
+        
+        * khtml/editing/htmlediting.cpp:
+        Removed unused include of dom_positioniterator.h and "using" of PositionIterator.
+        
+        * khtml/editing/selection.cpp:
+        Removed unused include of dom_positioniterator.h.
+
+        * khtml/editing/visible_position.h:
+        * khtml/editing/visible_position.cpp:
+        (khtml::VisiblePosition::previousVisiblePosition):
+        (khtml::VisiblePosition::nextVisiblePosition):
+        (khtml::VisiblePosition::downstreamDeepEquivalent):
+        Use Position::next(), Position::previous(), Position::atStart(), Position::atEnd() instead of duplicated code.
+        
+        * khtml/xml/dom_nodeimpl.h:
+        * khtml/xml/dom_nodeimpl.cpp:
+        (NodeImpl::maxDeepOffset):
+        Added to support Position::next(), Position::previous(), Position::atStart(), Position::atEnd()
+        
+        * khtml/xml/dom_position.h:
+        * khtml/xml/dom_position.cpp:
+        (DOM::Position::previous):
+        (DOM::Position::next):
+        (DOM::Position::atStart):
+        (DOM::Position::atEnd):
+        Moved here, replacing VisiblePosition's duplicate and PositionIterator.  Fixed to
+        return positions in order and not skip positions.
+        
+        (DOM::Position::previousCharacterPosition):
+        (DOM::Position::nextCharacterPosition):
+        Use Position::next(), Position::previous(), Position::atStart(), Position::atEnd() instead of PositionIterator.
+
+        (DOM::isStreamer):
+        (DOM::Position::upstream):
+        (DOM::Position::downstream):
+        Use Position::next(), Position::previous(), Position::atStart(), Position::atEnd() instead of PositionIterator.
+
+        * khtml/xml/dom_positioniterator.cpp: Removed.
+        * khtml/xml/dom_positioniterator.h: Removed.
+        Removed in favor of Position::next(), Position::previous(), Position::atStart(), Position::atEnd()
+
 2005-05-05  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Darin.
index f6664600e2193b06cba0c9f5ce2c51b4309aac8e..e0edd370d8425dd0d49365ff345ea307c5608ffa 100644 (file)
                                93F199E508245E59001E9ABC,
                                93F199E608245E59001E9ABC,
                                93F199E708245E59001E9ABC,
-                               93F199E808245E59001E9ABC,
                                93F199E908245E59001E9ABC,
                                93F199EA08245E59001E9ABC,
                                93F199EB08245E59001E9ABC,
                        settings = {
                        };
                };
-               93F199E808245E59001E9ABC = {
-                       fileRef = BE02F482066E1C550013A9F6;
-                       isa = PBXBuildFile;
-                       settings = {
-                       };
-               };
                93F199E908245E59001E9ABC = {
                        fileRef = BE02D4E6066F908A0076809F;
                        isa = PBXBuildFile;
                                93F19AFD08245E59001E9ABC,
                                93F19AFE08245E59001E9ABC,
                                93F19AFF08245E59001E9ABC,
-                               93F19B0008245E59001E9ABC,
                                93F19B0108245E59001E9ABC,
                                93F19B0208245E59001E9ABC,
                                93F19B0308245E59001E9ABC,
                        settings = {
                        };
                };
-               93F19B0008245E59001E9ABC = {
-                       fileRef = BE02F483066E1C550013A9F6;
-                       isa = PBXBuildFile;
-                       settings = {
-                       };
-               };
                93F19B0108245E59001E9ABC = {
                        fileRef = BE02D4E7066F908A0076809F;
                        isa = PBXBuildFile;
                        refType = 4;
                        sourceTree = "<group>";
                };
-               BE02F482066E1C550013A9F6 = {
-                       fileEncoding = 30;
-                       isa = PBXFileReference;
-                       lastKnownFileType = sourcecode.c.h;
-                       path = dom_positioniterator.h;
-                       refType = 4;
-                       sourceTree = "<group>";
-               };
-               BE02F483066E1C550013A9F6 = {
-                       fileEncoding = 30;
-                       isa = PBXFileReference;
-                       lastKnownFileType = sourcecode.cpp.cpp;
-                       path = dom_positioniterator.cpp;
-                       refType = 4;
-                       sourceTree = "<group>";
-               };
                BE16801805EDB91A00B87935 = {
                        fileEncoding = 30;
                        isa = PBXFileReference;
                                F523D2FA02DE4476018635CA,
                                BE91FC8B06133666005E3790,
                                BE91FC8C06133666005E3790,
-                               BE02F482066E1C550013A9F6,
-                               BE02F483066E1C550013A9F6,
                                F523D2FB02DE4476018635CA,
                                F523D2FC02DE4476018635CA,
                                F523D2FD02DE4476018635CA,
index 6a1ac46dca0a5ccd314859d30f62f539f252b376..c0eb391804d5ecac23ce093fd049dce87079cd16 100644 (file)
@@ -42,7 +42,6 @@
 #include "xml/dom_docimpl.h"
 #include "xml/dom_elementimpl.h"
 #include "xml/dom_nodeimpl.h"
-#include "xml/dom_positioniterator.h"
 #include "xml/dom_textimpl.h"
 #include "xml/dom2_rangeimpl.h"
 
index eb5355f1a10f36618537353ef1665954f269fd31..9e8d98effaa69b84fb0f2da05ed91683206ec7f1 100644 (file)
@@ -35,7 +35,6 @@
 #include "dom_elementimpl.h"
 #include "dom_nodeimpl.h"
 #include "dom_position.h"
-#include "dom_positioniterator.h"
 #include "dom_stringimpl.h"
 #include "dom_textimpl.h"
 #include "dom2_range.h"
@@ -83,7 +82,6 @@ using DOM::Node;
 using DOM::NodeImpl;
 using DOM::NodeListImpl;
 using DOM::Position;
-using DOM::PositionIterator;
 using DOM::Range;
 using DOM::RangeImpl;
 using DOM::StayInBlock;
index 6a1ac46dca0a5ccd314859d30f62f539f252b376..c0eb391804d5ecac23ce093fd049dce87079cd16 100644 (file)
@@ -42,7 +42,6 @@
 #include "xml/dom_docimpl.h"
 #include "xml/dom_elementimpl.h"
 #include "xml/dom_nodeimpl.h"
-#include "xml/dom_positioniterator.h"
 #include "xml/dom_textimpl.h"
 #include "xml/dom2_rangeimpl.h"
 
index 7bc4b0730b9a95a862544c96b8ca7812d4f64f7e..ae03b7f01e846c50ca7b33718e2301d08b06819e 100644 (file)
@@ -44,6 +44,7 @@
 using DOM::CharacterDataImpl;
 using DOM::NodeImpl;
 using DOM::offsetInCharacters;
+using DOM::UsingComposedCharacters;
 using DOM::Position;
 using DOM::Range;
 using DOM::RangeImpl;
@@ -184,7 +185,7 @@ VisiblePosition VisiblePosition::previous() const
 
 Position VisiblePosition::previousVisiblePosition(const Position &pos)
 {
-    if (pos.isNull() || atStart(pos))
+    if (pos.isNull() || pos.atStart())
         return Position();
 
     Position test = deepEquivalent(pos);
@@ -192,8 +193,8 @@ Position VisiblePosition::previousVisiblePosition(const Position &pos)
     bool acceptAnyVisiblePosition = !isCandidate(test);
 
     Position current = test;
-    while (!atStart(current)) {
-        current = previousPosition(current);
+    while (!current.atStart()) {
+        current = current.previous(UsingComposedCharacters);
         if (isCandidate(current) && (acceptAnyVisiblePosition || (downstreamTest != current.downstream(StayInBlock)))) {
             return current;
         }
@@ -204,7 +205,7 @@ Position VisiblePosition::previousVisiblePosition(const Position &pos)
 
 Position VisiblePosition::nextVisiblePosition(const Position &pos)
 {
-    if (pos.isNull() || atEnd(pos))
+    if (pos.isNull() || pos.atEnd())
         return Position();
 
     Position test = deepEquivalent(pos);
@@ -212,8 +213,8 @@ Position VisiblePosition::nextVisiblePosition(const Position &pos)
 
     Position current = test;
     Position downstreamTest = test.downstream(StayInBlock);
-    while (!atEnd(current)) {
-        current = nextPosition(current);
+    while (!current.atEnd()) {
+        current = current.next(UsingComposedCharacters);
         if (isCandidate(current) && (acceptAnyVisiblePosition || (downstreamTest != current.downstream(StayInBlock)))) {
             return current;
         }
@@ -222,62 +223,6 @@ Position VisiblePosition::nextVisiblePosition(const Position &pos)
     return Position();
 }
 
-Position VisiblePosition::previousPosition(const Position &pos)
-{
-    if (pos.isNull())
-        return pos;
-    
-    Position result;
-
-    if (pos.offset() <= 0) {
-        NodeImpl *prevNode = pos.node()->traversePreviousNode();
-        if (prevNode)
-            result = Position(prevNode, prevNode->maxOffset());
-    }
-    else {
-        NodeImpl *node = pos.node();
-        result = Position(node, node->previousOffset(pos.offset()));
-    }
-    
-    return result;
-}
-
-Position VisiblePosition::nextPosition(const Position &pos)
-{
-    if (pos.isNull())
-        return pos;
-    
-    Position result;
-
-    if (pos.offset() >= pos.node()->maxOffset()) {
-        NodeImpl *nextNode = pos.node()->traverseNextNode();
-        if (nextNode)
-            result = Position(nextNode, 0);
-    }
-    else {
-        NodeImpl *node = pos.node();
-        result = Position(node, node->nextOffset(pos.offset()));
-    }
-    
-    return result;
-}
-
-bool VisiblePosition::atStart(const Position &pos)
-{
-    if (pos.isNull())
-        return true;
-
-    return pos.offset() <= 0 && pos.node()->previousLeafNode() == 0;
-}
-
-bool VisiblePosition::atEnd(const Position &pos)
-{
-    if (pos.isNull())
-        return true;
-
-    return pos.offset() >= pos.node()->maxOffset() && pos.node()->nextLeafNode() == 0;
-}
-
 bool VisiblePosition::isCandidate(const Position &pos)
 {
     if (pos.isNull())
@@ -362,14 +307,14 @@ Position VisiblePosition::downstreamDeepEquivalent() const
 {
     Position pos = m_deepPosition;
     
-    if (pos.isNull() || atEnd(pos))
+    if (pos.isNull() || pos.atEnd())
         return pos;
 
     Position downstreamTest = pos.downstream(StayInBlock);
 
     Position current = pos;
-    while (!atEnd(current)) {
-        current = nextPosition(current);
+    while (!current.atEnd()) {
+        current = current.next(UsingComposedCharacters);
         if (isCandidate(current)) {
             if (downstreamTest != current.downstream(StayInBlock))
                 break;
index 63b65ab80d4e23a5c5be4f36d25d3b66b37f3fca..67a95fedd2814ffa50eaf5a4ca49746189ff29a2 100644 (file)
@@ -103,12 +103,6 @@ private:
     static Position previousVisiblePosition(const Position &);
     static Position nextVisiblePosition(const Position &);
 
-    static Position previousPosition(const Position &);
-    static Position nextPosition(const Position &);
-
-    static bool atStart(const Position &);
-    static bool atEnd(const Position &);
-
     static bool isCandidate(const Position &);
         
     Position m_deepPosition;
index ed8ea96d03b34acd143dc04fc4a07e0e4323317a..5ca66d289c5bf55c2fd3793607c244fd4b721c19 100644 (file)
@@ -1391,6 +1391,18 @@ long NodeImpl::maxOffset() const
     return 1;
 }
 
+// method for editing madness, which allows BR,1 as a position, though that is incorrect
+long NodeImpl::maxDeepOffset() const
+{
+    if (offsetInCharacters(nodeType()))
+        return static_cast<const TextImpl*>(this)->length();
+        
+    if (id() == ID_BR || (renderer() && renderer()->isReplaced()))
+        return 1;
+
+    return childNodeCount();
+}
+
 long NodeImpl::caretMinOffset() const
 {
     return renderer() ? renderer()->caretMinOffset() : 0;
index 307d17f96c4272527397e559aa10b2ad7c990e06..a77e6ea8a63686c0bf414c6e1eaed5da469ec30e 100644 (file)
@@ -363,6 +363,7 @@ public:
     virtual bool childAllowed( NodeImpl *newChild );
 
     virtual long maxOffset() const;
+    long maxDeepOffset() const;
     virtual long caretMinOffset() const;
     virtual long caretMaxOffset() const;
     virtual unsigned long caretMaxRenderedOffset() const;
index 2016d751e27fd3c71284a80dcded857c93b6a9b0..fcefa0ccab3490b951f2d33da38b7f80d11c7854 100644 (file)
@@ -31,7 +31,6 @@
 #include "css_valueimpl.h"
 #include "dom_elementimpl.h"
 #include "dom_nodeimpl.h"
-#include "dom_positioniterator.h"
 #include "dom2_range.h"
 #include "dom2_rangeimpl.h"
 #include "dom2_viewsimpl.h"
@@ -160,6 +159,82 @@ CSSComputedStyleDeclarationImpl *Position::computedStyle() const
     return new CSSComputedStyleDeclarationImpl(elem);
 }
 
+Position Position::previous(EUsingComposedCharacters usingComposedCharacters) const
+{
+    NodeImpl *n = node();
+    if (!n)
+        return *this;
+    
+    long o = offset();
+    assert(o >= 0);
+
+    if (o > 0) {
+        NodeImpl *child = n->childNode(o - 1);
+        if (child) {
+            return Position(child, child->maxDeepOffset());
+        }
+        // There are two reasons child might be 0:
+        //   1) The node is node like a text node that is not an element, and therefore has no children.
+        //      Going backward one character at a time is correct.
+        //   2) The old offset was a bogus offset like (<br>, 1), and there is no child.
+        //      Going from 1 to 0 is correct.
+        return Position(n, usingComposedCharacters ? n->previousOffset(o) : o - 1);
+    }
+
+    NodeImpl *parent = n->parentNode();
+    if (!parent)
+        return *this;
+
+    return Position(parent, n->nodeIndex());
+}
+
+Position Position::next(EUsingComposedCharacters usingComposedCharacters) const
+{
+    NodeImpl *n = node();
+    if (!n)
+        return *this;
+    
+    long o = offset();
+    assert(o >= 0);
+
+    if (o < n->maxDeepOffset()) {
+        NodeImpl *child = n->childNode(o);
+        if (child) {
+            return Position(child, 0);
+        }
+        // There are two reasons child might be 0:
+        //   1) The node is node like a text node that is not an element, and therefore has no children.
+        //      Going forward one character at a time is correct.
+        //   2) The new offset is a bogus offset like (<br>, 1), and there is no child.
+        //      Going from 0 to 1 is correct.
+        return Position(n, usingComposedCharacters ? n->nextOffset(o) : o + 1);
+    }
+
+    NodeImpl *parent = n->parentNode();
+    if (!parent)
+        return *this;
+
+    return Position(parent, n->nodeIndex() + 1);
+}
+
+bool Position::atStart() const
+{
+    NodeImpl *n = node();
+    if (!n)
+        return true;
+    
+    return offset() <= 0 && n->parent() == 0;
+}
+
+bool Position::atEnd() const
+{
+    NodeImpl *n = node();
+    if (!n)
+        return true;
+    
+    return offset() >= n->maxDeepOffset() && n->parent() == 0;
+}
+
 long Position::renderedOffset() const
 {
     if (!node()->isTextNode())
@@ -184,196 +259,273 @@ long Position::renderedOffset() const
     return result;
 }
 
+// return first preceding DOM position rendered at a different location, or "this"
 Position Position::previousCharacterPosition(EAffinity affinity) const
 {
     if (isNull())
         return Position();
 
     NodeImpl *fromRootEditableElement = node()->rootEditableElement();
-    PositionIterator it(*this);
 
     bool atStartOfLine = isFirstVisiblePositionOnLine(VisiblePosition(*this, affinity));
     bool rendered = inRenderedContent();
     
-    while (!it.atStart()) {
-        Position pos = it.previous();
+    Position currentPos = *this;
+    while (!currentPos.atStart()) {
+        currentPos = currentPos.previous();
 
-        if (pos.node()->rootEditableElement() != fromRootEditableElement)
+        if (currentPos.node()->rootEditableElement() != fromRootEditableElement)
             return *this;
 
         if (atStartOfLine || !rendered) {
-            if (pos.inRenderedContent())
-                return pos;
-        }
-        else if (rendersInDifferentPosition(pos))
-            return pos;
+            if (currentPos.inRenderedContent())
+                return currentPos;
+        } else if (rendersInDifferentPosition(currentPos))
+            return currentPos;
     }
     
     return *this;
 }
 
+// return first following position rendered at a different location, or "this"
 Position Position::nextCharacterPosition(EAffinity affinity) const
 {
     if (isNull())
         return Position();
 
     NodeImpl *fromRootEditableElement = node()->rootEditableElement();
-    PositionIterator it(*this);
 
     bool atEndOfLine = isLastVisiblePositionOnLine(VisiblePosition(*this, affinity));
     bool rendered = inRenderedContent();
     
-    while (!it.atEnd()) {
-        Position pos = it.next();
+    Position currentPos = *this;
+    while (!currentPos.atEnd()) {
+        currentPos = currentPos.next();
 
-        if (pos.node()->rootEditableElement() != fromRootEditableElement)
+        if (currentPos.node()->rootEditableElement() != fromRootEditableElement)
             return *this;
 
         if (atEndOfLine || !rendered) {
-            if (pos.inRenderedContent())
-                return pos;
-        }
-        else if (rendersInDifferentPosition(pos))
-            return pos;
+            if (currentPos.inRenderedContent())
+                return currentPos;
+        } else if (rendersInDifferentPosition(currentPos))
+            return currentPos;
     }
     
     return *this;
 }
 
+// upstream() and downstream() want to return positions that are either in a
+// text node or at just before a non-text node.  This method checks for that.
+static bool     isStreamer (Position pos)
+{
+    NodeImpl *currentNode = pos.node();
+    if (!currentNode)
+        return true;
+        
+    if (currentNode->isAtomicNode())
+        return true;
+        
+    return (pos.offset() == 0);
+}
+
+// AFAIK no one has a clear, complete definition for this method and how it is used.
+// Here is what I have come to understand from re-working the code after fixing PositionIterator
+// for <rdar://problem/4103339>.  See also Ken's comments in the header.  Fundamentally, upstream()
+// scans backward in the DOM starting at "this" to return a visible DOM position that is either in
+// a text node, or just after a replaced or BR element (btw downstream() also considers empty blocks).
+// If "stayInBlock" is specified, the search stops when it would have entered into a part of the DOM
+// with a different enclosing block, including a nested one.  Otherwise, the search stops at the start
+// of the entire DOM tree.  If "stayInBlock" stops the search, this method returns the highest previous
+// position that is either in an atomic node (i.e. text) or is the end of a non-atomic node
+// (_regardless_ of visibility).  If the end-of-DOM stopped the search, this method returns the 
+// highest previous visible node that is either in an atomic node (i.e. text) or is the end of a
+// non-atomic node.
 Position Position::upstream(EStayInBlock stayInBlock) const
 {
+    // start at equivalent deep position
     Position start = equivalentDeepPosition();
     NodeImpl *startNode = start.node();
     if (!startNode)
         return Position();
-
-    NodeImpl *block = startNode->enclosingBlockFlowOrTableElement();
-    Position lastVisible;
     
-    PositionIterator it(start);
-    for (; !it.atStart(); it.previous()) {
-        NodeImpl *currentNode = it.current().node();
-
-        if (stayInBlock) {
-            NodeImpl *currentBlock = currentNode->enclosingBlockFlowOrTableElement();
-            if (block != currentBlock)
-                return it.next();
-        }
-
+    // iterate backward from there, looking for a qualified position
+    NodeImpl *block = stayInBlock ? startNode->enclosingBlockFlowOrTableElement() : 0;
+    Position lastVisible = *this;
+    Position lastStreamer = *this;
+    Position currentPos = start;
+    for (; !currentPos.atStart(); currentPos = currentPos.previous()) {
+        NodeImpl *currentNode = currentPos.node();
+        int currentOffset = currentPos.offset();
+
+        // limit traversal to block or table enclosing the original element
+        // NOTE: This includes not going into nested blocks
+        if (stayInBlock && block != currentNode->enclosingBlockFlowOrTableElement())
+            return lastStreamer;
+
+        // track last streamer position (regardless of visibility)
+        if (isStreamer(currentPos))
+            lastStreamer = currentPos;
+
+        // skip position in unrendered or invisible node
         RenderObject *renderer = currentNode->renderer();
-        if (!renderer)
+        if (!renderer || renderer->style()->visibility() != VISIBLE)
             continue;
 
-        if (renderer->style()->visibility() != VISIBLE)
-            continue;
-
-        lastVisible = it.current();
+        // track last visible streamer position
+        if (isStreamer(currentPos))
+            lastVisible = currentPos;
 
+        // return position after replaced or BR elements
         if (renderer->isReplaced() || renderer->isBR()) {
-            if (it.current().offset() >= renderer->caretMaxOffset())
+            if (currentOffset >= renderer->caretMaxOffset())
                 return Position(currentNode, renderer->caretMaxOffset());
-            else
-                continue;
+
+            // we could not have iterated here because we would have returned
+            // this node, caretMaxOffset, so we must have started here
+            assert(currentPos == start);
+            continue;
         }
 
+        // return current position if it is in rendered text
         if (renderer->isText() && static_cast<RenderText *>(renderer)->firstTextBox()) {
-            if (currentNode != startNode)
+            if (currentNode != startNode) {
+                assert(currentOffset >= renderer->caretMaxOffset());
                 return Position(currentNode, renderer->caretMaxOffset());
+            }
 
-            if (it.current().offset() < 0)
+            if (currentOffset < 0)
                 continue;
-            uint textOffset = it.current().offset();
 
+            uint textOffset = currentOffset;
             RenderText *textRenderer = static_cast<RenderText *>(renderer);
             for (InlineTextBox *box = textRenderer->firstTextBox(); box; box = box->nextTextBox()) {
                 if (textOffset > box->start() && textOffset <= box->start() + box->len())
-                    return it.current();
-                else if (box != textRenderer->lastTextBox() && 
-                         !box->nextOnLine() && 
-                         textOffset == box->start() + box->len() + 1)
-                    return it.current();
+                    return currentPos;
+                    
+                if (box != textRenderer->lastTextBox() && 
+                    !box->nextOnLine() && 
+                    textOffset == box->start() + box->len() + 1)
+                    return currentPos;
             }
         }
     }
-    
-    return lastVisible.isNotNull() ? lastVisible : *this;
+
+    return lastVisible;
 }
 
+// AFAIK no one has a clear, complete definition for this method and how it is used.
+// Here is what I have come to understand from re-working the code after fixing PositionIterator
+// for <rdar://problem/4103339>.  See also Ken's comments in the header.  Fundamentally, downstream()
+// scans forward in the DOM starting at "this" to return the first visible DOM position that is
+// either in a text node, or just before a replaced, BR element, or empty block flow element (i.e.
+// non-text nodes with no children).  If "stayInBlock" is specified, the search stops when it would
+// have entered into a part of the DOM with a different enclosing block, including a nested one.
+// Otherwise, the search stops at the end of the entire DOM tree.  If "stayInBlock" stops the search,
+// this method returns the first previous position that is either in an atomic node (i.e. text) or is
+// at offset 0 (_regardless_ of visibility).  If the end-of-DOM stopped the search, this method returns
+// the first previous visible node that is either in an atomic node (i.e. text) or is at offset 0.
 Position Position::downstream(EStayInBlock stayInBlock) const
 {
+    // start at equivalent deep position
     Position start = equivalentDeepPosition();
     NodeImpl *startNode = start.node();
     if (!startNode)
         return Position();
 
-    NodeImpl *block = startNode->enclosingBlockFlowOrTableElement();
-    Position lastVisible;
-    
-    PositionIterator it(start);            
-    for (; !it.atEnd(); it.next()) {   
-        NodeImpl *currentNode = it.current().node();
-
-        if (stayInBlock) {
-            NodeImpl *currentBlock = currentNode->enclosingBlockFlowOrTableElement();
-            if (block != currentBlock)
-                return it.previous();
-        }
+    // iterate forward from there, looking for a qualified position
+    NodeImpl *block = stayInBlock ? startNode->enclosingBlockFlowOrTableElement() : 0;
+    Position lastVisible = *this;
+    Position lastStreamer = *this;
+    Position currentPos = start;
+    for (; !currentPos.atEnd(); currentPos = currentPos.next()) {   
+        NodeImpl *currentNode = currentPos.node();
+        int currentOffset = currentPos.offset();
+
+        // stop before going above the body, up into the head
+        // return the last visible streamer position
+        if (currentNode->id() == ID_BODY && currentOffset >= (int) currentNode->childNodeCount())
+            break;
+            
+        // limit traversal to block or table enclosing the original element
+        // return the last streamer position regardless of visibility
+        // NOTE: This includes not going into nested blocks
+        if (stayInBlock && block != currentNode->enclosingBlockFlowOrTableElement())
+            return lastStreamer;
+        
+        // track last streamer position (regardless of visibility)
+        if (isStreamer(currentPos))
+            lastStreamer = currentPos;
 
+        // skip position in unrendered or invisible node
         RenderObject *renderer = currentNode->renderer();
-        if (!renderer)
-            continue;
-
-        if (renderer->style()->visibility() != VISIBLE)
+        if (!renderer || renderer->style()->visibility() != VISIBLE)
             continue;
+        
+        // track last visible streamer position
+        if (isStreamer(currentPos))
+            lastVisible = currentPos;
+
+        // if now at a offset 0 of a rendered block flow element...
+        //      - return current position if the element has no children (i.e. is a leaf)
+        //      - return child node, offset 0, if the first visible child is not a block flow element
+        //      - otherwise, skip this position (first visible child is a block, and we will
+        //          get there eventually via the iterator)
+        if ((currentNode != startNode && renderer->isBlockFlow()) && (currentOffset == 0)) {
+            if (!currentNode->firstChild())
+                return currentPos;
+                
+            for (NodeImpl *child = currentNode->firstChild(); child; child = child->nextSibling()) {
+                RenderObject *r = child->renderer();
+                if (r && r->style()->visibility() == VISIBLE) {
+                    if (r->isBlockFlow())
+                        break; // break causes continue code below to run.
 
-        lastVisible = it.current();
-
-        if (currentNode != startNode && renderer->isBlockFlow()) {
-            if (it.current().offset() == 0) {
-                // If no first child, or first visible child is a not a block, return; otherwise continue.
-                if (!currentNode->firstChild())
-                    return Position(currentNode, 0);
-                for (NodeImpl *child = currentNode->firstChild(); child; child = child->nextSibling()) {
-                    RenderObject *r = child->renderer();
-                    if (r && r->style()->visibility() == VISIBLE) {
-                         if (r->isBlockFlow())
-                            break; // break causes continue code below to run.
-                         else
-                            return Position(child, 0);
-                    }
+                    return Position(child, 0);
                 }
-                continue;
             }
+
+            continue;
         }
 
+        // return position before replaced or BR elements
         if (renderer->isReplaced() || renderer->isBR()) {
-            if (it.current().offset() <= renderer->caretMinOffset())
+            if (currentOffset <= renderer->caretMinOffset())
                 return Position(currentNode, renderer->caretMinOffset());
-            else
-                continue;
+            
+            // we could not have iterated here because we would have returned
+            // this node, offset 0, so we must have started here
+            assert(currentPos == start);
+            continue;
         }
 
+        // return current position if it is in rendered text
         if (renderer->isText() && static_cast<RenderText *>(renderer)->firstTextBox()) {
-            if (currentNode != start.node())
+            if (currentNode != startNode) {
+                assert(currentOffset == 0);
                 return Position(currentNode, renderer->caretMinOffset());
+            }
 
-            if (it.current().offset() < 0)
+            if (currentOffset < 0)
                 continue;
-            uint textOffset = it.current().offset();
+
+            uint textOffset = currentOffset;
 
             RenderText *textRenderer = static_cast<RenderText *>(renderer);
             for (InlineTextBox *box = textRenderer->firstTextBox(); box; box = box->nextTextBox()) {
                 if (textOffset >= box->start() && textOffset <= box->end())
-                    return it.current();
-                else if (box != textRenderer->lastTextBox() && 
-                         !box->nextOnLine() && 
-                         textOffset == box->start() + box->len())
-                    return it.current();
+                    return currentPos;
+                
+                if (box != textRenderer->lastTextBox() && 
+                     !box->nextOnLine() && 
+                     textOffset == box->start() + box->len()) {
+                    return currentPos;
+                }
             }
         }
     }
     
-    return lastVisible.isNotNull() ? lastVisible : *this;
+    return lastVisible;
 }
 
 Position Position::equivalentRangeCompliantPosition() const
index a9b5251c071e19081a70943cd031751f7b1ffca9..ee095df8e98b0c2aca9fc6de37746b4b23d93c32 100644 (file)
@@ -37,6 +37,7 @@ class Range;
 class RangeImpl;
 
 enum EStayInBlock { DoNotStayInBlock = false, StayInBlock = true };
+enum EUsingComposedCharacters { NotUsingComposedCharacters = false, UsingComposedCharacters = true };
 
 class Position
 {
@@ -59,6 +60,12 @@ public:
     ElementImpl *element() const;
     CSSComputedStyleDeclarationImpl *computedStyle() const;
 
+    // Move up or down the DOM by one position
+    Position previous(EUsingComposedCharacters usingComposedCharacters=NotUsingComposedCharacters) const;
+    Position next(EUsingComposedCharacters usingComposedCharacters=NotUsingComposedCharacters) const;
+    bool atStart() const;
+    bool atEnd() const;
+    
     // FIXME: Make these non-member functions and put them somewhere in the editing directory.
     // These aren't really basic "position" operations. More high level editing helper functions.
     Position leadingWhitespacePosition(khtml::EAffinity affinity, bool considerNonCollapsibleWhitespace = false) const;
diff --git a/WebCore/khtml/xml/dom_positioniterator.cpp b/WebCore/khtml/xml/dom_positioniterator.cpp
deleted file mode 100644 (file)
index 8b5362d..0000000
+++ /dev/null
@@ -1,88 +0,0 @@
-/*
- * Copyright (C) 2004 Apple Computer, Inc.  All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
- * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
- * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
- * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
- */
-
-#include "dom_positioniterator.h"
-
-#include "dom_nodeimpl.h"
-
-namespace DOM {
-
-Position PositionIterator::peekPrevious() const
-{
-    Position pos = m_current;
-    
-    if (pos.isNull())
-        return Position();
-    
-    if (pos.offset() <= 0) {
-        NodeImpl *prevNode = pos.node()->traversePreviousNode();
-        if (prevNode)
-            pos = Position(prevNode, prevNode->maxOffset());
-    }
-    else {
-        pos = Position(pos.node(), pos.offset() - 1);
-    }
-    
-    return pos;
-}
-
-Position PositionIterator::peekNext() const
-{
-    Position pos = m_current;
-    
-    if (pos.isNull())
-        return Position();
-    
-    if (pos.offset() >= pos.node()->maxOffset()) {
-        NodeImpl *nextNode = pos.node()->traverseNextNode();
-        if (nextNode)
-            pos = Position(nextNode, 0);
-    }
-    else {
-        pos = Position(pos.node(), pos.offset() + 1);
-    }
-    
-    return pos;
-}
-
-bool PositionIterator::atStart() const
-{
-    if (m_current.isNull())
-        return true;
-
-    return m_current.offset() == 0 && 
-        m_current.node()->traversePreviousNode() == 0;
-}
-
-bool PositionIterator::atEnd() const
-{
-    if (m_current.isNull())
-        return true;
-
-    return m_current.offset() >= m_current.node()->maxOffset() && 
-        m_current.node()->traverseNextNode() == 0;
-}
-
-} // namespace DOM
diff --git a/WebCore/khtml/xml/dom_positioniterator.h b/WebCore/khtml/xml/dom_positioniterator.h
deleted file mode 100644 (file)
index cd67119..0000000
+++ /dev/null
@@ -1,61 +0,0 @@
-/*
- * Copyright (C) 2004 Apple Computer, Inc.  All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
- * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
- * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
- * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
- */
-
-#ifndef _PositionIterator_h_
-#define _PositionIterator_h_
-
-#include "dom_position.h"
-
-namespace DOM {
-
-class NodeImpl;
-class Position;
-
-class PositionIterator
-{
-public:
-    PositionIterator() : m_current() {}
-    PositionIterator(NodeImpl *node, long offset) : m_current(node, offset) {}
-    PositionIterator(const Position &o) : m_current(o) {}
-
-    Position current() const { return m_current; }
-    Position previous() { return m_current = peekPrevious(); }
-    Position next() { return m_current = peekNext(); }
-    Position peekPrevious() const;
-    Position peekNext() const;
-
-    void setPosition(const Position &pos) { m_current = pos; }
-
-    bool atStart() const;
-    bool atEnd() const;
-    bool isEmpty() const { return m_current.isNull(); }
-
-private:
-    Position m_current;
-};
-
-} // namespace DOM
-
-#endif // _PositionIterator_h_