LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Oct 2006 01:37:04 +0000 (01:37 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Oct 2006 01:37:04 +0000 (01:37 +0000)
        Reviewed by darin

        <http://bugs.webkit.org/show_bug.cgi?id=10993>
        GMail Editor: Caret doesn't always position itself after list marker

        * editing/execCommand/create-list-1-expected.checksum: Added.
        * editing/execCommand/create-list-1-expected.png: Added.
        * editing/execCommand/create-list-1-expected.txt: Added.
        * editing/execCommand/create-list-1.html: Added.

        Fixed:
        * editing/execCommand/create-list-with-hr-expected.checksum:
        * editing/execCommand/create-list-with-hr-expected.png:
        * editing/execCommand/create-list-with-hr-expected.txt:
        * editing/execCommand/create-list-with-hr.html:
        * editing/execCommand/remove-list-1-expected.checksum:
        * editing/execCommand/remove-list-1-expected.png:
        * editing/execCommand/remove-list-1-expected.txt:
        * fast/text/attributed-substring-from-range-001-expected.txt:

WebCore:

        Reviewed by darin

        <http://bugs.webkit.org/show_bug.cgi?id=10993>
        GMail Editor: Caret doesn't always position itself after list marker

        List creation uses moveParagraphs to push content into list items.
        moveParagraphs uses a TextIterator to restore selections after moves.
        Some characters emitted by the TextIterator had bad ranges associated
        with them.  rangeFromLocationAndLength would skip past the range it
        should have used when asked for ranges of length 0.

        * editing/TextIterator.cpp:
        (WebCore::TextIterator::TextIterator): No longer need to initialize a
        removed member variable.
        (WebCore::TextIterator::advance): An extra newline is emitted when leaving
        some blocks.  Use the same range for this newline as for the first newline.
        We should remove this code and just emit two '\n's.
        (WebCore::TextIterator::handleTextNode): Setup m_range.
        (WebCore::TextIterator::handleTextBox): Ditto.
        (WebCore::TextIterator::handleReplacedElement): Ditto.
        (WebCore::TextIterator::handleNonTextNode): Ditto.
        (WebCore::TextIterator::exitNode): Use an m_range from the last VisiblePosition
        in the block we're leaving to that VP after that one.
        (WebCore::TextIterator::emitCharacter): This function now takes in the start
        and the end of the range associated with the emited character, and sets up m_range.
        (WebCore::TextIterator::range): Return m_range.  If it is null (we are atEnd),
        return the end of the range used to create the iterator, as a convenience to
        callers that use call range() on an iterator that is atEnd.
        (WebCore::SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator):
        Same as the changes made to TextIterator's constructor.
        (WebCore::SimplifiedBackwardsTextIterator::advance): Use a null m_range instead of
        a null m_positionNode to signal that we're finished.
        (WebCore::SimplifiedBackwardsTextIterator::handleTextNode): Ditto.
        (WebCore::SimplifiedBackwardsTextIterator::handleReplacedElement): Similar to changes
        made to TextIterator.
        (WebCore::SimplifiedBackwardsTextIterator::emitCharacter): Ditto.
        (WebCore::SimplifiedBackwardsTextIterator::emitNewline): Simplified.
        (WebCore::SimplifiedBackwardsTextIterator::range): Similar to the changes made to
        TextIterator::range.
        (WebCore::CharacterIterator::range): This function assumed that an iterator's
        range() was safe to modify.
        (WebCore::TextIterator::rangeFromLocationAndLength):
        If the range we're looking for starts in the current chunk, this function assumed
        that if the chunk started in a text node, it would end in the same text node.  This
        is no longer the case.
        If the range we're looking for starts in the middle of the current chunk, I assume
        that the chunk is inside a text node, because those are the only chunks with length
        greater than one at the moment.
        If the range we're looking for is a zero length range that starts/ends at the end of the
        current chunk, we used to return the start of the next chunk, but that's wrong and
        is what caused this bug.
        * editing/TextIterator.h:
        (WebCore::TextIterator::atEnd): The iterator is atEnd when m_range is null.
        (WebCore::SimplifiedBackwardsTextIterator::atEnd):
        * editing/visible_units.cpp:
        (WebCore::previousBoundary): Cleaned up by using a convenience function.
        (WebCore::nextBoundary): Ditto.

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

17 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/execCommand/create-list-1-expected.checksum [new file with mode: 0644]
LayoutTests/editing/execCommand/create-list-1-expected.png [new file with mode: 0644]
LayoutTests/editing/execCommand/create-list-1-expected.txt [new file with mode: 0644]
LayoutTests/editing/execCommand/create-list-1.html [new file with mode: 0644]
LayoutTests/editing/execCommand/create-list-with-hr-expected.checksum
LayoutTests/editing/execCommand/create-list-with-hr-expected.png
LayoutTests/editing/execCommand/create-list-with-hr-expected.txt
LayoutTests/editing/execCommand/create-list-with-hr.html
LayoutTests/editing/execCommand/remove-list-1-expected.checksum
LayoutTests/editing/execCommand/remove-list-1-expected.png
LayoutTests/editing/execCommand/remove-list-1-expected.txt
LayoutTests/fast/text/attributed-substring-from-range-001-expected.txt
WebCore/ChangeLog
WebCore/editing/TextIterator.cpp
WebCore/editing/TextIterator.h
WebCore/editing/visible_units.cpp

index 32042e775df5764253abefb5fb5f3eb731548483..6b7928761229f932a7a84335b39cbd9bd4379f10 100644 (file)
@@ -1,3 +1,25 @@
+2006-10-24  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+
+        <http://bugs.webkit.org/show_bug.cgi?id=10993>
+        GMail Editor: Caret doesn't always position itself after list marker
+
+        * editing/execCommand/create-list-1-expected.checksum: Added.
+        * editing/execCommand/create-list-1-expected.png: Added.
+        * editing/execCommand/create-list-1-expected.txt: Added.
+        * editing/execCommand/create-list-1.html: Added.
+        
+        Fixed:
+        * editing/execCommand/create-list-with-hr-expected.checksum:
+        * editing/execCommand/create-list-with-hr-expected.png:
+        * editing/execCommand/create-list-with-hr-expected.txt:
+        * editing/execCommand/create-list-with-hr.html:
+        * editing/execCommand/remove-list-1-expected.checksum:
+        * editing/execCommand/remove-list-1-expected.png:
+        * editing/execCommand/remove-list-1-expected.txt:
+        * fast/text/attributed-substring-from-range-001-expected.txt:
+        
 2006-10-23  Justin Garcia  <justin.garcia@apple.com>
 
         Reviewed by mjs
diff --git a/LayoutTests/editing/execCommand/create-list-1-expected.checksum b/LayoutTests/editing/execCommand/create-list-1-expected.checksum
new file mode 100644 (file)
index 0000000..70d0df3
--- /dev/null
@@ -0,0 +1 @@
+b2623aa776b910eb80f202c78ef53d5e
\ No newline at end of file
diff --git a/LayoutTests/editing/execCommand/create-list-1-expected.png b/LayoutTests/editing/execCommand/create-list-1-expected.png
new file mode 100644 (file)
index 0000000..c38e38a
Binary files /dev/null and b/LayoutTests/editing/execCommand/create-list-1-expected.png differ
diff --git a/LayoutTests/editing/execCommand/create-list-1-expected.txt b/LayoutTests/editing/execCommand/create-list-1-expected.txt
new file mode 100644 (file)
index 0000000..a9086a8
--- /dev/null
@@ -0,0 +1,29 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 4 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of LI > OL > DIV > BODY > HTML > #document to 0 of LI > OL > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {P} at (0,0) size 784x18
+        RenderText {#text} at (0,0) size 539x18
+          text run at (0,0) width 248: "This tests list creation in an empty line. "
+          text run at (248,0) width 291: "The caret should end up in the empty list item."
+      RenderBlock {DIV} at (0,34) size 784x86
+        RenderBlock (anonymous) at (0,0) size 784x18
+          RenderText {#text} at (0,0) size 99x18
+            text run at (0,0) width 99: "Paragraph One."
+          RenderBR {BR} at (99,14) size 0x0
+        RenderBlock {OL} at (0,34) size 784x18
+          RenderListItem {LI} at (40,0) size 744x18
+            RenderListMarker at (-20,0) size 16x18
+        RenderBlock (anonymous) at (0,68) size 784x18
+          RenderText {#text} at (0,0) size 102x18
+            text run at (0,0) width 102: "Paragraph Two."
+caret: position 0 of child 0 {LI} of child 2 {OL} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/execCommand/create-list-1.html b/LayoutTests/editing/execCommand/create-list-1.html
new file mode 100644 (file)
index 0000000..19006e8
--- /dev/null
@@ -0,0 +1,11 @@
+<p>This tests list creation in an empty line.  The caret should end up in the empty list item.</p>
+<div contenteditable="true" id="div">Paragraph One.<br><br>Paragraph Two.</div>
+
+<script>
+var div = document.getElementById("div");
+var sel = window.getSelection();
+
+sel.setPosition(div, 0);
+sel.modify("move", "forward", "line");
+document.execCommand("InsertOrderedList");
+</script>
index 728fa147cc6341a0e71cb0ed33be33761b0c70e5..f738018c157210260fbcbb832f99494272673044 100644 (file)
@@ -1 +1 @@
-1a920b25e56e869eaa3be7e22932cd09
\ No newline at end of file
+f557f6e45eb1c0240e2e0e6edfa6b0cc
\ No newline at end of file
index 3595703a4e3b3bdc829718ae1d8aa9651cc1bccf..a423abe902a4cf3f3e0de0e7875f50d01a98f857 100644 (file)
Binary files a/LayoutTests/editing/execCommand/create-list-with-hr-expected.png and b/LayoutTests/editing/execCommand/create-list-with-hr-expected.png differ
index 660a931f394431d99d2e8bfb360b229469fda0fb..797d7533cb37f2621cf7699b617a82ad96f76955 100644 (file)
@@ -2,7 +2,7 @@ EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML
 EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 49 of #text > B > P > BODY > HTML > #document to 49 of #text > B > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > LI > UL > DIV > BODY > HTML > #document to 0 of DIV > LI > UL > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
@@ -10,18 +10,14 @@ layer at (0,0) size 800x600
 layer at (0,0) size 800x600
   RenderBlock {HTML} at (0,0) size 800x600
     RenderBody {BODY} at (8,8) size 784x576
-      RenderBlock {P} at (0,0) size 784x54
+      RenderBlock {P} at (0,0) size 784x36
         RenderText {#text} at (0,0) size 514x18
           text run at (0,0) width 514: "This test pushes a horizontal rule into an unordered list with InsertUnorderedList. "
         RenderInline {B} at (0,0) size 771x36
           RenderText {#text} at (514,0) size 771x36
             text run at (514,0) width 257: "The fact that the horizontal rule is put"
-            text run at (0,18) width 544: "into an unnecessary div when it's pushed into the list might be considered a bug. "
-        RenderInline {B} at (0,0) size 726x36
-          RenderText {#text} at (544,18) size 726x36
-            text run at (544,18) width 182: "The fact that the caret isn't"
-            text run at (0,36) width 127: "preserved is a bug."
-      RenderBlock {DIV} at (0,70) size 784x28
+            text run at (0,18) width 540: "into an unnecessary div when it's pushed into the list might be considered a bug."
+      RenderBlock {DIV} at (0,52) size 784x28
         RenderBlock {UL} at (0,0) size 784x28
           RenderListItem {LI} at (40,0) size 744x28
             RenderBlock (anonymous) at (0,0) size 744x18
@@ -30,4 +26,4 @@ layer at (0,0) size 800x600
               RenderBlock {HR} at (0,0) size 744x2 [border: (1px inset #000000)]
             RenderBlock (anonymous) at (0,36) size 744x0
         RenderBlock (anonymous) at (0,44) size 784x0
-caret: position 49 of child 0 {#text} of child 2 {B} of child 0 {P} of child 0 {BODY} of child 0 {HTML} of document
+caret: position 0 of child 0 {HR} of child 0 {DIV} of child 0 {LI} of child 0 {UL} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
index 5a26da12f6d437993f389a058aadd82e0559ccda..0408032d1da52f02e7203515668a8d202c660fac 100644 (file)
@@ -1,4 +1,4 @@
-<p>This test pushes a horizontal rule into an unordered list with InsertUnorderedList. <b>The fact that the horizontal rule is put into an unnecessary div when it's pushed into the list might be considered a bug. </b><b>The fact that the caret isn't preserved is a bug.</b></p>
+<p>This test pushes a horizontal rule into an unordered list with InsertUnorderedList. <b>The fact that the horizontal rule is put into an unnecessary div when it's pushed into the list might be considered a bug.</b></p>
 <div contenteditable="true" id="div"><hr></div>
 
 <script>
index 9aebb0e2638847ab3670b658f7103bf4e6cfa0f0..f8a2259e346d236ba739df95b60392a45a3fd87e 100644 (file)
@@ -1 +1 @@
-958726dee6c5c5c73089d158c46f17c5
\ No newline at end of file
+2b441a195ceda3a506ff2bb266cbe8a2
\ No newline at end of file
index 0cf1c88640bbe9b0a98dc4e38d7b689a7724f66a..80bdf66abe39350bfa4ec0adeb0ae42c32c19ae3 100644 (file)
Binary files a/LayoutTests/editing/execCommand/remove-list-1-expected.png and b/LayoutTests/editing/execCommand/remove-list-1-expected.png differ
index 520d167db23627a27a67ccaa45ccdfdb508ac45c..179d10c5d8f15c9365a677e19ff925e0cdebe28f 100644 (file)
@@ -4,7 +4,7 @@ EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of LI > OL > DIV > BODY > HTML > #document to 49 of #text > LI > OL > DIV > BODY > HTML > #document toDOMRange:range from 0 of LI > OL > DIV > BODY > HTML > #document to 49 of #text > LI > OL > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > BODY > HTML > #document to 49 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
@@ -23,4 +23,5 @@ layer at (0,0) size 800x600
         RenderBR {BR} at (0,0) size 0x18
         RenderText {#text} at (0,18) size 322x18
           text run at (0,18) width 322: "There should be a empty paragraph above this one."
-caret: position 0 of child 0 {BR} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+selection start: position 0 of child 0 {BR} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+selection end:   position 49 of child 1 {#text} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
index cdfbcb0361561bf8a50a1318948ca70ce3e8b7ee..87b6df9c0530fe642d9c05cfd10afc4d09bfba5b 100644 (file)
@@ -8,9 +8,9 @@ Testing text with br elements (<div><br></div>hello<br>)
 (0, 2): {}h{ NSColor = NSCalibratedWhiteColorSpace 0 1; NSFont = "Times-Roman 16.00 pt. S [] (0xXXXXXXXX) fobj=0xXXXXXXXX, spc=4.00"; }
 (1, 1): h{ NSColor = NSCalibratedWhiteColorSpace 0 1; NSFont = "Times-Roman 16.00 pt. S [] (0xXXXXXXXX) fobj=0xXXXXXXXX, spc=4.00"; }
 (1, 5): hello{ NSColor = NSCalibratedWhiteColorSpace 0 1; NSFont = "Times-Roman 16.00 pt. S [] (0xXXXXXXXX) fobj=0xXXXXXXXX, spc=4.00"; }
-(1, 6): hello{ NSColor = NSCalibratedWhiteColorSpace 0 1; NSFont = "Times-Roman 16.00 pt. S [] (0xXXXXXXXX) fobj=0xXXXXXXXX, spc=4.00"; } {}
-(1, 100): hello{ NSColor = NSCalibratedWhiteColorSpace 0 1; NSFont = "Times-Roman 16.00 pt. S [] (0xXXXXXXXX) fobj=0xXXXXXXXX, spc=4.00"; } {}
-(6, 1): {}
+(1, 6): hello{ NSColor = NSCalibratedWhiteColorSpace 0 1; NSFont = "Times-Roman 16.00 pt. S [] (0xXXXXXXXX) fobj=0xXXXXXXXX, spc=4.00"; }
+(1, 100): hello{ NSColor = NSCalibratedWhiteColorSpace 0 1; NSFont = "Times-Roman 16.00 pt. S [] (0xXXXXXXXX) fobj=0xXXXXXXXX, spc=4.00"; }
+(6, 1):
 (7, 1):
 (100, 0): undefined
 (100, 100): undefined
index dc0a844c3899abc351a73ca113b09c51f237b0fa..91fd01fce8f3b3c7bbe450e83b02f725682c2550 100644 (file)
@@ -1,3 +1,63 @@
+2006-10-24  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+
+        <http://bugs.webkit.org/show_bug.cgi?id=10993>
+        GMail Editor: Caret doesn't always position itself after list marker
+        
+        List creation uses moveParagraphs to push content into list items.  
+        moveParagraphs uses a TextIterator to restore selections after moves.
+        Some characters emitted by the TextIterator had bad ranges associated
+        with them.  rangeFromLocationAndLength would skip past the range it
+        should have used when asked for ranges of length 0.
+
+        * editing/TextIterator.cpp:
+        (WebCore::TextIterator::TextIterator): No longer need to initialize a
+        removed member variable.
+        (WebCore::TextIterator::advance): An extra newline is emitted when leaving
+        some blocks.  Use the same range for this newline as for the first newline.
+        We should remove this code and just emit two '\n's.
+        (WebCore::TextIterator::handleTextNode): Setup m_range.
+        (WebCore::TextIterator::handleTextBox): Ditto.
+        (WebCore::TextIterator::handleReplacedElement): Ditto.
+        (WebCore::TextIterator::handleNonTextNode): Ditto.
+        (WebCore::TextIterator::exitNode): Use an m_range from the last VisiblePosition
+        in the block we're leaving to that VP after that one.
+        (WebCore::TextIterator::emitCharacter): This function now takes in the start
+        and the end of the range associated with the emited character, and sets up m_range.
+        (WebCore::TextIterator::range): Return m_range.  If it is null (we are atEnd), 
+        return the end of the range used to create the iterator, as a convenience to
+        callers that use call range() on an iterator that is atEnd.
+        (WebCore::SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator):
+        Same as the changes made to TextIterator's constructor.
+        (WebCore::SimplifiedBackwardsTextIterator::advance): Use a null m_range instead of
+        a null m_positionNode to signal that we're finished.
+        (WebCore::SimplifiedBackwardsTextIterator::handleTextNode): Ditto.
+        (WebCore::SimplifiedBackwardsTextIterator::handleReplacedElement): Similar to changes
+        made to TextIterator.
+        (WebCore::SimplifiedBackwardsTextIterator::emitCharacter): Ditto.
+        (WebCore::SimplifiedBackwardsTextIterator::emitNewline): Simplified.
+        (WebCore::SimplifiedBackwardsTextIterator::range): Similar to the changes made to
+        TextIterator::range.
+        (WebCore::CharacterIterator::range): This function assumed that an iterator's
+        range() was safe to modify.
+        (WebCore::TextIterator::rangeFromLocationAndLength):
+        If the range we're looking for starts in the current chunk, this function assumed
+        that if the chunk started in a text node, it would end in the same text node.  This
+        is no longer the case.
+        If the range we're looking for starts in the middle of the current chunk, I assume
+        that the chunk is inside a text node, because those are the only chunks with length
+        greater than one at the moment.
+        If the range we're looking for is a zero length range that starts/ends at the end of the
+        current chunk, we used to return the start of the next chunk, but that's wrong and
+        is what caused this bug.
+        * editing/TextIterator.h:
+        (WebCore::TextIterator::atEnd): The iterator is atEnd when m_range is null.
+        (WebCore::SimplifiedBackwardsTextIterator::atEnd):
+        * editing/visible_units.cpp:
+        (WebCore::previousBoundary): Cleaned up by using a convenience function.
+        (WebCore::nextBoundary): Ditto.
+
 2006-10-24  Anders Carlsson  <acarlsson@apple.com>
 
         Reviewed by Maciej.
index b073a2c59e930eb102139e9b24d7507f36f34071..fe2cecf02ca277e5e33c265ace8e7c39787dc159 100644 (file)
@@ -73,11 +73,11 @@ private:
     CircularSearchBuffer &operator=(const CircularSearchBuffer&);
 };
 
-TextIterator::TextIterator() : m_endContainer(0), m_endOffset(0), m_positionNode(0), m_lastCharacter(0)
+TextIterator::TextIterator() : m_endContainer(0), m_endOffset(0), m_lastCharacter(0)
 {
 }
 
-TextIterator::TextIterator(const Range *r, IteratorKind kind) : m_endContainer(0), m_endOffset(0), m_positionNode(0)
+TextIterator::TextIterator(const Range *r, IteratorKind kind) : m_endContainer(0), m_endOffset(0)
 {
     if (!r)
         return;
@@ -120,8 +120,8 @@ TextIterator::TextIterator(const Range *r, IteratorKind kind) : m_endContainer(0
         m_lastCharacter = '\n';
 
 #ifndef NDEBUG
-    // need this just because of the assert in advance()
-    m_positionNode = m_node;
+    // Need this just because of the assert in advance().
+    m_range = new Range(m_node->document(), Position(m_node, 0), Position(m_node, 0));
 #endif
 
     // identify the first run
@@ -131,13 +131,16 @@ TextIterator::TextIterator(const Range *r, IteratorKind kind) : m_endContainer(0
 void TextIterator::advance()
 {
     // reset the run information
-    m_positionNode = 0;
+    m_range = 0;
     m_textLength = 0;
 
     // handle remembered node that needed a newline after the text node's newline
     if (m_needAnotherNewline) {
-        // emit the newline, with position a collapsed range at the end of current node.
-        emitCharacter('\n', m_node->parentNode(), m_node, 1, 1);
+        VisiblePosition lastInNode(Position(m_node, maxDeepOffset(m_node)));
+        VisiblePosition next = lastInNode.next();
+        Position start = lastInNode.deepEquivalent();
+        Position end = next.isNull() ? start : next.deepEquivalent();
+        emitCharacter('\n', start, end);
         m_needAnotherNewline = false;
         return;
     }
@@ -145,7 +148,7 @@ void TextIterator::advance()
     // handle remembered text box
     if (m_textBox) {
         handleTextBox();
-        if (m_positionNode)
+        if (m_range)
             return;
     }
 
@@ -162,7 +165,7 @@ void TextIterator::advance()
                     m_handledNode = handleReplacedElement();
             } else
                 m_handledNode = handleNonTextNode();
-            if (m_positionNode)
+            if (m_range)
                 return;
         }
 
@@ -178,7 +181,7 @@ void TextIterator::advance()
                 while (!next && m_node->parentNode()) {
                     m_node = m_node->parentNode();
                     exitNode();
-                    if (m_positionNode) {
+                    if (m_range) {
                         m_handledNode = true;
                         m_handledChildren = true;
                         return;
@@ -193,8 +196,8 @@ void TextIterator::advance()
         m_handledNode = false;
         m_handledChildren = false;
 
-        // how would this ever be?
-        if (m_positionNode)
+        // FIXME: How would this ever be?
+        if (m_range)
             return;
     }
 }
@@ -215,7 +218,7 @@ bool TextIterator::handleTextNode()
     if (!renderer->style()->collapseWhiteSpace()) {
         int runStart = m_offset;
         if (m_lastTextNodeEndedWithCollapsedSpace) {
-            emitCharacter(' ', m_node, 0, runStart, runStart);
+            emitCharacter(' ', Position(m_node, runStart), Position(m_node, runStart));
             return false;
         }
         int strLength = str.length();
@@ -225,10 +228,8 @@ bool TextIterator::handleTextNode()
         if (runStart >= runEnd)
             return true;
 
-        m_positionNode = m_node;
-        m_positionOffsetBaseNode = 0;
-        m_positionStartOffset = runStart;
-        m_positionEndOffset = runEnd;
+        m_range = new Range(m_node->document(), Position(m_node, runStart), Position(m_node, runEnd));
+        
         m_textCharacters = str.characters() + runStart;
         m_textLength = runEnd - runStart;
 
@@ -272,7 +273,7 @@ void TextIterator::handleTextBox()
         bool needSpace = m_lastTextNodeEndedWithCollapsedSpace
             || (m_textBox == firstTextBox && textBoxStart == runStart && runStart > 0);
         if (needSpace && !isCollapsibleWhitespace(m_lastCharacter) && m_lastCharacter) {
-            emitCharacter(' ', m_node, 0, runStart, runStart);
+            emitCharacter(' ', Position(m_node, runStart), Position(m_node, runStart));
             return;
         }
         int textBoxEnd = textBoxStart + m_textBox->m_len;
@@ -291,7 +292,7 @@ void TextIterator::handleTextBox()
             // or a run of characters that does not include a newline.
             // This effectively translates newlines to spaces without copying the text.
             if (str[runStart] == '\n') {
-                emitCharacter(' ', m_node, 0, runStart, runStart + 1);
+                emitCharacter(' ', Position(m_node, runStart), Position(m_node, runStart + 1));
                 m_offset = runStart + 1;
             } else {
                 int subrunEnd = str.find('\n', runStart);
@@ -300,10 +301,7 @@ void TextIterator::handleTextBox()
 
                 m_offset = subrunEnd;
 
-                m_positionNode = m_node;
-                m_positionOffsetBaseNode = 0;
-                m_positionStartOffset = runStart;
-                m_positionEndOffset = subrunEnd;
+                m_range = new Range(m_node->document(), Position(m_node, runStart), Position(m_node, subrunEnd));
                 m_textCharacters = str.characters() + runStart;
                 m_textLength = subrunEnd - runStart;
 
@@ -313,7 +311,8 @@ void TextIterator::handleTextBox()
 
             // If we are doing a subrun that doesn't go to the end of the text box,
             // come back again to finish handling this text box; don't advance to the next one.
-            if (m_positionEndOffset < textBoxEnd)
+            ExceptionCode ec;
+            if (m_range->endOffset(ec) < textBoxEnd)
                 return;
 
             // Advance and return
@@ -335,14 +334,11 @@ void TextIterator::handleTextBox()
 bool TextIterator::handleReplacedElement()
 {
     if (m_lastTextNodeEndedWithCollapsedSpace) {
-        emitCharacter(' ', m_lastTextNode->parentNode(), m_lastTextNode, 1, 1);
+        emitCharacter(' ', positionAfterNode(m_lastTextNode), positionAfterNode(m_lastTextNode));
         return false;
     }
 
-    m_positionNode = m_node->parentNode();
-    m_positionOffsetBaseNode = m_node;
-    m_positionStartOffset = 0;
-    m_positionEndOffset = 1;
+    m_range = new Range(m_node->document(), positionBeforeNode(m_node), positionAfterNode(m_node));
 
     m_textCharacters = 0;
     m_textLength = 0;
@@ -470,15 +466,20 @@ static bool shouldEmitExtraNewlineForNode(Node* node)
 bool TextIterator::handleNonTextNode()
 {
     if (shouldEmitNewlineForNode(m_node)) {
-        emitCharacter('\n', m_node->parentNode(), m_node, 0, 1);
+        Position start = positionBeforeNode(m_node);
+        VisiblePosition next = VisiblePosition(start).next();
+        Position end = next.isNull() ? start : next.deepEquivalent();
+        emitCharacter('\n', start, end);
     } else if (m_lastCharacter != '\n' && m_lastTextNode) {
-        // only add the tab or newline if not at the start of a line
+        // Only add the tab or newline if not at the start of a line.
+        // FIXME: The ranges for these emitted characters should be the part of the 
+        // document that this imaginary character would occupy.
         if (shouldEmitTabBeforeNode(m_node))
-            emitCharacter('\t', m_lastTextNode->parentNode(), m_lastTextNode, 0, 1);
+            emitCharacter('\t', positionBeforeNode(m_lastTextNode), positionAfterNode(m_lastTextNode));
         else if (shouldEmitNewlinesBeforeAndAfterNode(m_node))
-            emitCharacter('\n', m_lastTextNode->parentNode(), m_lastTextNode, 0, 1);
+            emitCharacter('\n', positionBeforeNode(m_lastTextNode), positionAfterNode(m_lastTextNode));
         else if (shouldEmitSpaceBeforeAndAfterNode(m_node))
-            emitCharacter(' ', m_lastTextNode->parentNode(), m_lastTextNode, 0, 1);
+            emitCharacter(' ', positionBeforeNode(m_lastTextNode), positionAfterNode(m_lastTextNode));
     }
 
     return true;
@@ -486,33 +487,33 @@ bool TextIterator::handleNonTextNode()
 
 void TextIterator::exitNode()
 {
+    VisiblePosition lastInNode(Position(m_node, maxDeepOffset(m_node)));
+    VisiblePosition next = lastInNode.next();
+    Position start = lastInNode.deepEquivalent();
+    Position end = next.isNull() ? start : next.deepEquivalent();
+    
     if (m_lastTextNode && shouldEmitNewlinesBeforeAndAfterNode(m_node)) {
         // use extra newline to represent margin bottom, as needed
         bool addNewline = shouldEmitExtraNewlineForNode(m_node);
         
         if (m_lastCharacter != '\n') {
-            // insert a newline with a position following this block
-            emitCharacter('\n', m_node->parentNode(), m_node, 1, 1);
+
+            emitCharacter('\n', start, end);
 
             // remember whether to later add a newline for the current node
             assert(!m_needAnotherNewline);
             m_needAnotherNewline = addNewline;
         } else if (addNewline) {
             // insert a newline with a position following this block
-            emitCharacter('\n', m_node->parentNode(), m_node, 1, 1);
+            emitCharacter('\n', start, end);
         }
     } else if (shouldEmitSpaceBeforeAndAfterNode(m_node))
-        emitCharacter(' ', m_node->parentNode(), m_node, 1, 1);
+        emitCharacter(' ', start, end);
 }
 
-void TextIterator::emitCharacter(UChar c, Node *textNode, Node *offsetBaseNode, int textStartOffset, int textEndOffset)
+void TextIterator::emitCharacter(UChar c, const Position& start, const Position& end)
 {
-    // remember information with which to construct the TextIterator::range()
-    // NOTE: textNode is often not a text node, so the range will specify child nodes of positionNode
-    m_positionNode = textNode;
-    m_positionOffsetBaseNode = offsetBaseNode;
-    m_positionStartOffset = textStartOffset;
-    m_positionEndOffset = textEndOffset;
+    m_range = start.isNull() ? 0 : new Range(start.node()->document(), start, end);
  
     // remember information with which to construct the TextIterator::characters() and length()
     m_singleCharacterBuffer = c;
@@ -526,32 +527,21 @@ void TextIterator::emitCharacter(UChar c, Node *textNode, Node *offsetBaseNode,
 
 PassRefPtr<Range> TextIterator::range() const
 {
-    // use the current run information, if we have it
-    if (m_positionNode) {
-        if (m_positionOffsetBaseNode) {
-            int index = m_positionOffsetBaseNode->nodeIndex();
-            m_positionStartOffset += index;
-            m_positionEndOffset += index;
-            m_positionOffsetBaseNode = 0;
-        }
-        return new Range(m_positionNode->document(), m_positionNode, m_positionStartOffset, m_positionNode, m_positionEndOffset);
-    }
-
-    // otherwise, return the end of the overall range we were given
+    if (m_range)
+        return m_range;
+        
     if (m_endContainer)
         return new Range(m_endContainer->document(), m_endContainer, m_endOffset, m_endContainer, m_endOffset);
-        
+    
     return 0;
 }
 
-SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator() : m_positionNode(0)
+SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator()
 {
 }
 
 SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator(const Range *r)
 {
-    m_positionNode = 0;
-
     if (!r)
         return;
 
@@ -592,7 +582,7 @@ SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator(const Range *r)
 
 #ifndef NDEBUG
     // Need this just because of the assert.
-    m_positionNode = endNode;
+    m_range = new Range(endNode->document(), Position(endNode, 0), Position(endNode, 0));
 #endif
 
     m_lastTextNode = 0;
@@ -603,9 +593,9 @@ SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator(const Range *r)
 
 void SimplifiedBackwardsTextIterator::advance()
 {
-    assert(m_positionNode);
+    assert(m_range);
 
-    m_positionNode = 0;
+    m_range = 0;
     m_textLength = 0;
 
     while (m_node) {
@@ -620,7 +610,7 @@ void SimplifiedBackwardsTextIterator::advance()
                     m_handledNode = handleReplacedElement();
             } else
                 m_handledNode = handleNonTextNode();
-            if (m_positionNode)
+            if (m_range)
                 return;
         }
 
@@ -668,7 +658,7 @@ void SimplifiedBackwardsTextIterator::advance()
             m_offset = 0;
         m_handledNode = false;
         
-        if (m_positionNode)
+        if (m_range)
             return;
     }
 }
@@ -683,26 +673,21 @@ bool SimplifiedBackwardsTextIterator::handleTextNode()
     if (!renderer->firstTextBox() && str.length() > 0)
         return true;
 
-    m_positionEndOffset = m_offset;
-
+    Position end(m_node, m_offset);
     m_offset = (m_node == m_startNode) ? m_startOffset : 0;
-    m_positionNode = m_node;
-    m_positionStartOffset = m_offset;
-    m_textLength = m_positionEndOffset - m_positionStartOffset;
-    m_textCharacters = str.characters() + m_positionStartOffset;
+    Position start(m_node, m_offset);
+    m_range = new Range(m_node->document(), start, end);
+    m_textLength = end.offset() - start.offset();
+    m_textCharacters = str.characters() + start.offset();
 
-    m_lastCharacter = str[m_positionEndOffset - 1];
+    m_lastCharacter = str[end.offset() - 1];
 
     return true;
 }
 
 bool SimplifiedBackwardsTextIterator::handleReplacedElement()
 {
-    int offset = m_node->nodeIndex();
-
-    m_positionNode = m_node->parentNode();
-    m_positionStartOffset = offset;
-    m_positionEndOffset = offset + 1;
+    m_range = new Range(m_node->document(), positionBeforeNode(m_node), positionAfterNode(m_node));
 
     m_textCharacters = 0;
     m_textLength = 0;
@@ -734,12 +719,12 @@ void SimplifiedBackwardsTextIterator::exitNode()
     handleNonTextNode();
 }
 
-void SimplifiedBackwardsTextIterator::emitCharacter(UChar c, Node *node, int startOffset, int endOffset)
+void SimplifiedBackwardsTextIterator::emitCharacter(UChar c, const Position& start, const Position& end)
 {
     m_singleCharacterBuffer = c;
-    m_positionNode = node;
-    m_positionStartOffset = startOffset;
-    m_positionEndOffset = endOffset;
+    
+    m_range = start.isNull() ? 0 : new Range(start.node()->document(), start, end);
+    
     m_textCharacters = &m_singleCharacterBuffer;
     m_textLength = 1;
     m_lastCharacter = c;
@@ -747,21 +732,16 @@ void SimplifiedBackwardsTextIterator::emitCharacter(UChar c, Node *node, int sta
 
 void SimplifiedBackwardsTextIterator::emitNewline()
 {
-    int offset;
-    
-    if (m_lastTextNode) {
-        offset = m_lastTextNode->nodeIndex();
-        emitCharacter('\n', m_lastTextNode->parentNode(), offset, offset + 1);
-    } else {
-        offset = m_node->nodeIndex();
-        emitCharacter('\n', m_node->parentNode(), offset, offset + 1);
-    }
+    if (m_lastTextNode)
+        emitCharacter('\n', positionBeforeNode(m_lastTextNode), positionAfterNode(m_lastTextNode));
+    else
+        emitCharacter('\n', positionBeforeNode(m_node), positionAfterNode(m_node));
 }
 
 PassRefPtr<Range> SimplifiedBackwardsTextIterator::range() const
 {
-    if (m_positionNode)
-        return new Range(m_positionNode->document(), m_positionNode, m_positionStartOffset, m_positionNode, m_positionEndOffset);
+    if (m_range)
+        return m_range;
     
     return new Range(m_startNode->document(), m_startNode, m_startOffset, m_startNode, m_startOffset);
 }
@@ -781,7 +761,8 @@ CharacterIterator::CharacterIterator(const Range *r)
 
 PassRefPtr<Range> CharacterIterator::range() const
 {
-    RefPtr<Range> r = m_textIterator.range();
+    ExceptionCode ec;
+    RefPtr<Range> r = m_textIterator.range()->cloneRange(ec);
     if (!m_textIterator.atEnd()) {
         if (m_textIterator.length() <= 1) {
             assert(m_runOffset == 0);
@@ -1051,14 +1032,11 @@ PassRefPtr<Range> TextIterator::rangeFromLocationAndLength(Element *scope, int r
         if (rangeLocation >= docTextPosition && rangeLocation <= docTextPosition + len) {
             startRangeFound = true;
             int exception = 0;
-            if (textRunRange->startContainer(exception)->isTextNode()) {
+            if (rangeLocation == docTextPosition + len)
+                resultRange->setStart(textRunRange->endContainer(exception), textRunRange->endOffset(exception), exception);
+            else {
                 int offset = rangeLocation - docTextPosition;
                 resultRange->setStart(textRunRange->startContainer(exception), offset + textRunRange->startOffset(exception), exception);
-            } else {
-                if (rangeLocation == docTextPosition)
-                    resultRange->setStart(textRunRange->startContainer(exception), textRunRange->startOffset(exception), exception);
-                else
-                    resultRange->setStart(textRunRange->endContainer(exception), textRunRange->endOffset(exception), exception);
             }
         }
 
@@ -1073,10 +1051,9 @@ PassRefPtr<Range> TextIterator::rangeFromLocationAndLength(Element *scope, int r
                 else
                     resultRange->setEnd(textRunRange->endContainer(exception), textRunRange->endOffset(exception), exception);
             }
-            if (!(rangeLength == 0 && rangeEnd == docTextPosition + len)) {
-                docTextPosition += len;
-                break;
-            }
+            
+            docTextPosition += len;
+            break;
         }
         docTextPosition += len;
     }
index f11cc0cedabee1295b8c922b80f935f5c1545d8f..cf9d3c80338a8ab3f9d5c4265c69d0a92fb615ae 100644 (file)
@@ -62,7 +62,7 @@ public:
     TextIterator();
     explicit TextIterator(const Range *, IteratorKind kind = CONTENT );
     
-    bool atEnd() const { return !m_positionNode; }
+    bool atEnd() const { return !m_range; }
     void advance();
     
     int length() const { return m_textLength; }
@@ -79,7 +79,7 @@ private:
     bool handleReplacedElement();
     bool handleNonTextNode();
     void handleTextBox();
-    void emitCharacter(UChar, Node *textNode, Node *offsetBaseNode, int textStartOffset, int textEndOffset);
+    void emitCharacter(UChar, const Position&, const Position&);
     
     // Current position, not necessarily of the text being returned, but position
     // as we walk through the DOM tree.
@@ -94,10 +94,7 @@ private:
     Node *m_pastEndNode;
     
     // The current text and its position, in the form to be returned from the iterator.
-    Node *m_positionNode;
-    mutable Node *m_positionOffsetBaseNode;
-    mutable int m_positionStartOffset;
-    mutable int m_positionEndOffset;
+    RefPtr<Range> m_range;
     const UChar* m_textCharacters;
     int m_textLength;
     
@@ -128,7 +125,7 @@ public:
     SimplifiedBackwardsTextIterator();
     explicit SimplifiedBackwardsTextIterator(const Range *);
     
-    bool atEnd() const { return !m_positionNode; }
+    bool atEnd() const { return !m_range; }
     void advance();
     
     int length() const { return m_textLength; }
@@ -141,7 +138,7 @@ private:
     bool handleTextNode();
     bool handleReplacedElement();
     bool handleNonTextNode();
-    void emitCharacter(UChar, Node *Node, int startOffset, int endOffset);
+    void emitCharacter(UChar, const Position&, const Position&);
     void emitNewline();
     
     // Current position, not necessarily of the text being returned, but position
@@ -156,9 +153,7 @@ private:
     int m_startOffset;
     
     // The current text and its position, in the form to be returned from the iterator.
-    Node* m_positionNode;
-    int m_positionStartOffset;
-    int m_positionEndOffset;
+    RefPtr<Range> m_range;
     const UChar* m_textCharacters;
     int m_textLength;
 
index ab33ca0f663f4f55f7807e4d4c8229f210593649..e9c8dc132d14bd53d3bb95617cd9cf4235d3094f 100644 (file)
@@ -85,8 +85,7 @@ static VisiblePosition previousBoundary(const VisiblePosition &c, unsigned (*sea
     }
     
     if (it.atEnd() && next == 0) {
-        RefPtr<Range> range(it.range());
-        pos = Position(range->startContainer(exception), range->startOffset(exception));
+        pos = it.range()->startPosition();
     } else if (!it.atEnd() && it.length() == 0) {
         // Got a zero-length chunk.
         // This means we have hit a replaced element.
@@ -161,9 +160,7 @@ static VisiblePosition nextBoundary(const VisiblePosition &c, unsigned (*searchF
     }
     
     if (it.atEnd() && next == string.length()) {
-        RefPtr<Range> range(it.range());
-        int exception = 0;
-        pos = Position(range->startContainer(exception), range->startOffset(exception));
+        pos = it.range()->startPosition();
     } else if (!it.atEnd() && it.length() == 0) {
         // Got a zero-length chunk.
         // This means we have hit a replaced element.
@@ -185,7 +182,7 @@ static VisiblePosition nextBoundary(const VisiblePosition &c, unsigned (*searchF
         // Use the character iterator to translate the next value into a DOM position.
         CharacterIterator charIt(searchRange.get());
         charIt.advance(next - 1);
-        pos = Position(charIt.range()->endContainer(ec), charIt.range()->endOffset(ec));
+        pos = charIt.range()->endPosition();
     }
 
     // generate VisiblePosition, use UPSTREAM affinity if possible