AX: Inconsistency between CharacterOffset and VisiblePostition
authorn_wang@apple.com <n_wang@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Feb 2016 18:58:31 +0000 (18:58 +0000)
committern_wang@apple.com <n_wang@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Feb 2016 18:58:31 +0000 (18:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154431

Reviewed by Chris Fleizach.

Source/WebCore:

VoiceOver is not getting the correct text marker from VisiblePostition when
navigating using arrow keys. We should make the CharacterOffset behavior consistent
with VisiblePosition so that the conversion between the two won't create different
text markers.

Changes are covered in the modified tests.

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::characterOffsetForTextMarkerData):
(WebCore::AXObjectCache::traverseToOffsetInRange):
(WebCore::AXObjectCache::startOrEndCharacterOffsetForRange):
(WebCore::AXObjectCache::startOrEndTextMarkerDataForRange):
(WebCore::AXObjectCache::characterOffsetForNodeAndOffset):
(WebCore::AXObjectCache::textMarkerDataForNextCharacterOffset):
(WebCore::AXObjectCache::textMarkerDataForPreviousCharacterOffset):
(WebCore::AXObjectCache::visiblePositionFromCharacterOffset):
(WebCore::AXObjectCache::characterOffsetFromVisiblePosition):
(WebCore::AXObjectCache::accessibilityObjectForTextMarkerData):
(WebCore::AXObjectCache::textMarkerDataForVisiblePosition):
(WebCore::AXObjectCache::nextCharacterOffset):
(WebCore::AXObjectCache::previousCharacterOffset):
(WebCore::AXObjectCache::startCharacterOffsetOfWord):
(WebCore::AXObjectCache::endCharacterOffsetOfWord):
(WebCore::AXObjectCache::previousWordStartCharacterOffset):
(WebCore::AXObjectCache::previousParagraphStartCharacterOffset):
(WebCore::AXObjectCache::previousSentenceStartCharacterOffset):
* accessibility/AXObjectCache.h:
* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(-[WebAccessibilityObjectWrapper doAXAttributedStringForTextMarkerRange:]):

LayoutTests:

* accessibility/mac/text-marker-word-nav-expected.txt:
* accessibility/mac/text-marker-word-nav.html:
* accessibility/text-marker/text-marker-previous-next.html:

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

LayoutTests/ChangeLog
LayoutTests/accessibility/mac/text-marker-word-nav-expected.txt
LayoutTests/accessibility/mac/text-marker-word-nav.html
LayoutTests/accessibility/text-marker/text-marker-previous-next.html
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AXObjectCache.cpp
Source/WebCore/accessibility/AXObjectCache.h
Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm

index 54cb5d1e5b2e029ba505356a63424072340bbce5..567c55c6e9605f296a4e7d9f7d4d1c5822b09fec 100644 (file)
@@ -1,3 +1,14 @@
+2016-02-19  Nan Wang  <n_wang@apple.com>
+
+        AX: Inconsistency between CharacterOffset and VisiblePostition
+        https://bugs.webkit.org/show_bug.cgi?id=154431
+
+        Reviewed by Chris Fleizach.
+
+        * accessibility/mac/text-marker-word-nav-expected.txt:
+        * accessibility/mac/text-marker-word-nav.html:
+        * accessibility/text-marker/text-marker-previous-next.html:
+
 2016-02-19  Ryan Haddad  <ryanhaddad@apple.com>
 
         Rebaseline imported/w3c/web-platform-tests/html/dom/interfaces.html for ios-simulator after r196797
index d64fa5c3ade3da2ec931fb596de3825f2682a6f2..b2edcd1f7c0cdc765f60d3d08693ec8eeb08f5ea 100644 (file)
@@ -19,6 +19,11 @@ Left word is: word1
 Right word is: word1
 Pre word start to next word end: word1
 
+Current character is: t
+Left word is: test
+Right word is: 
+Pre word start to next word end: test'line break'Thisislongword
+
 Current character is: T
 Left word is: Thisislongword
 Right word is: Thisislongword
@@ -86,17 +91,17 @@ Pre word start to next word end: both
 
 Current character is: s
 Left word is: spaces
-Right word is: 'line break'
+Right word is: 
 Pre word start to next word end: spaces'line break'
 
 Current character is: e
 Left word is: some
-Right word is: 'line break'
+Right word is: 
 Pre word start to next word end: some'line break'
 
 Current character is: 'line break'
-Left word is: 'line break'
-Right word is: text
+Left word is: 
+Right word is: 
 Pre word start to next word end: 'line break'text
 
 Current character is:  
index 3a9201c0b411504fbdc1774f2f1283e7c72cbddb..a376878f57e6fa97b47789f3e4499cdecbf6b5b3 100644 (file)
@@ -57,17 +57,20 @@ test audio <audio controls><source src="test.mp3" type="audio/mpeg"></audio>file
         var startMarker = text.startTextMarkerForTextMarkerRange(textMarkerRange);
         var currentMarker = advanceAndVerify(startMarker, 1, text);
         
+        // Check that we are at the end of paragraph, so right word should be empty
+        currentMarker = advanceAndVerify(currentMarker, 9, text);
+        
         // Check the case with span
         // At "T" in "Thisis", should return the word as "Thisislongword".
-        currentMarker = advanceAndVerify(currentMarker, 10, text);
+        currentMarker = advanceAndVerify(currentMarker, 2, text);
         // At " " before "I", the word should be "I'll".
-        currentMarker = advanceAndVerify(currentMarker, 14, text);
+        currentMarker = advanceAndVerify(currentMarker, 15, text);
         // At " " before "try", the word should excludes "."
-        currentMarker = advanceAndVerify(currentMarker, 5, text);
+        currentMarker = advanceAndVerify(currentMarker, 6, text);
         
         // Check the case with contenteditable
         // At "e" in "editable", the word should NOT include "Content" before it.
-        currentMarker = advanceAndVerify(currentMarker, 17, text);
+        currentMarker = advanceAndVerify(currentMarker, 19, text);
         
         // Check the case with replaced node, the replaced node should be considered a word.
         var text2 = accessibilityController.accessibleElementById("text2");
index 9ac6b2e818ec85ad0af56ea15abdd558521ea11f..33d2201c32b08275a2a781c0f846afa7c4572a6c 100644 (file)
@@ -49,31 +49,32 @@ f
         shouldBeTrue("text.accessibilityElementForTextMarker(endMarker).isEqual(text)");
         
         // Check next text marker. (Advance 5 characters, it will land at <br>.)
-        var currentMarker = startMarker;
-        var previousMarker, markerRange;
-        for (var i = 0; i < 5; i++) {
-            previousMarker = currentMarker;
-            currentMarker = text.nextTextMarker(currentMarker);
-        }
+        var currentMarker, previousMarker, markerRange;;
+        var result = forward(5, previousMarker, startMarker, text);
+        previousMarker = result.previous;
+        currentMarker = result.current;
         markerRange = text.textMarkerRangeForMarkers(previousMarker, currentMarker);
         var newline = '\n';
         shouldBe("text.stringForTextMarkerRange(markerRange)", "newline");
         
-        // Advance one more character, it will lande at "t" in "text1".
-        previousMarker = currentMarker;
-        currentMarker = text.nextTextMarker(currentMarker);
+        // Advance one more characters, it will lande at "t" in "text1".
+        result = forward(1, previousMarker, currentMarker, text);
+        previousMarker = result.previous;
+        currentMarker = result.current;
         markerRange = text.textMarkerRangeForMarkers(previousMarker, currentMarker);
         shouldBe("text.stringForTextMarkerRange(markerRange)", "'t'");
         
         // Check previous text marker. (Traverse backwards one character, it will land at <br>.)
-        previousMarker = text.previousTextMarker(previousMarker);
-        currentMarker = text.previousTextMarker(currentMarker);
+        result = backwards(1, previousMarker, currentMarker, text);
+        previousMarker = result.previous;
+        currentMarker = result.current;
         markerRange = text.textMarkerRangeForMarkers(previousMarker, currentMarker);
         shouldBe("text.stringForTextMarkerRange(markerRange)", "newline");
         
-        // Traverse backwards one more character, it will land at the last character of "text".
-        previousMarker = text.previousTextMarker(previousMarker);
-        currentMarker = text.previousTextMarker(currentMarker);
+        // Traverse backwards two more character, it will land at the last character of "text".
+        result = backwards(2, previousMarker, currentMarker, text);
+        previousMarker = result.previous;
+        currentMarker = result.current;
         markerRange = text.textMarkerRangeForMarkers(previousMarker, currentMarker);
         shouldBe("text.stringForTextMarkerRange(markerRange)", "'t'");
         
@@ -86,30 +87,29 @@ f
         
         currentMarker = text2.startTextMarkerForTextMarkerRange(textMarkerRange2);
         // Advance 5 characters, it will land at "d".
-        for (var i = 0; i < 5; i++) {
-            previousMarker = currentMarker;
-            currentMarker = text2.nextTextMarker(currentMarker);
-        }
+        result = forward(5, previousMarker, currentMarker, text2);
+        previousMarker = result.previous;
+        currentMarker = result.current;
         markerRange = text2.textMarkerRangeForMarkers(previousMarker, currentMarker);
         shouldBe("text2.stringForTextMarkerRange(markerRange)", "'d'");
         
-        // Traverse backwards 5 characters, it will land at the last character of "text1".
-        for (var i = 0; i < 5; i++) {
-            previousMarker = text2.previousTextMarker(previousMarker);
-            currentMarker = text2.previousTextMarker(currentMarker);
-        }
+        // Traverse backwards 8 characters, it will land at the last character of "text1".
+        result = backwards(8, previousMarker, currentMarker, text2);
+        previousMarker = result.previous;
+        currentMarker = result.current;
         markerRange = text2.textMarkerRangeForMarkers(previousMarker, currentMarker);
         shouldBe("text2.stringForTextMarkerRange(markerRange)", "'1'");
         
         // Check the case with user-select:none, nextTextMarker/previousTextMarker should still work.
         var text3 = accessibilityController.accessibleElementById("text3");
         text3 = text3.childAtIndex(0);
+        
         // Advance to land at user-select:none node.
         var marker1, marker2;
-        for (var i = 0; i < 17; i++) {
+        for (var i = 0; i < 18; i++) {
             currentMarker = text3.nextTextMarker(currentMarker);
-            // i == 13, it should land on "e", and i == 16, it should land on "t"
-            if (i == 13) {
+            // i == 14, it should land on "e", and i == 17, it should land on "t"
+            if (i == 14) {
                 marker1 = currentMarker;
             }
         }
@@ -139,6 +139,7 @@ f
         shouldBeTrue("text2.accessibilityElementForTextMarker(currentMarker).isEqual(text3)");
         // Check previous text marker, it should land on " d" node.
         currentMarker = text2.previousTextMarker(currentMarker);
+        currentMarker = text2.previousTextMarker(currentMarker);
         shouldBeTrue("text2.accessibilityElementForTextMarker(currentMarker).isEqual(text2.childAtIndex(2))");
         
         
@@ -162,6 +163,27 @@ f
         markerRange = text.textMarkerRangeForMarkers(startMarker, currentMarker)
         shouldBe("text.stringForTextMarkerRange(markerRange)", "'a'");
         
+        function forward(count, previousMarker, currentMarker, obj) {
+            for (var i = 0; i < count; i++) {
+                previousMarker = currentMarker;
+                currentMarker = obj.nextTextMarker(currentMarker);
+            }
+            return {
+                previous: previousMarker,
+                current: currentMarker
+            };
+        }
+        
+        function backwards(count, previousMarker, currentMarker, obj) {
+            for (var i = 0; i < count; i++) {
+                previousMarker = obj.previousTextMarker(previousMarker);
+                currentMarker = obj.previousTextMarker(currentMarker);
+            }
+            return {
+                previous: previousMarker,
+                current: currentMarker
+            };
+        }
     }
 
 </script>
index 60a6634c462aa3cc46af872e04ecf6a2bda34829..c8ffe9fc88774d11b0b7f6ab42e88956f4fd56d5 100644 (file)
@@ -1,3 +1,40 @@
+2016-02-19  Nan Wang  <n_wang@apple.com>
+
+        AX: Inconsistency between CharacterOffset and VisiblePostition
+        https://bugs.webkit.org/show_bug.cgi?id=154431
+
+        Reviewed by Chris Fleizach.
+
+        VoiceOver is not getting the correct text marker from VisiblePostition when
+        navigating using arrow keys. We should make the CharacterOffset behavior consistent
+        with VisiblePosition so that the conversion between the two won't create different
+        text markers.
+        
+        Changes are covered in the modified tests.
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::characterOffsetForTextMarkerData):
+        (WebCore::AXObjectCache::traverseToOffsetInRange):
+        (WebCore::AXObjectCache::startOrEndCharacterOffsetForRange):
+        (WebCore::AXObjectCache::startOrEndTextMarkerDataForRange):
+        (WebCore::AXObjectCache::characterOffsetForNodeAndOffset):
+        (WebCore::AXObjectCache::textMarkerDataForNextCharacterOffset):
+        (WebCore::AXObjectCache::textMarkerDataForPreviousCharacterOffset):
+        (WebCore::AXObjectCache::visiblePositionFromCharacterOffset):
+        (WebCore::AXObjectCache::characterOffsetFromVisiblePosition):
+        (WebCore::AXObjectCache::accessibilityObjectForTextMarkerData):
+        (WebCore::AXObjectCache::textMarkerDataForVisiblePosition):
+        (WebCore::AXObjectCache::nextCharacterOffset):
+        (WebCore::AXObjectCache::previousCharacterOffset):
+        (WebCore::AXObjectCache::startCharacterOffsetOfWord):
+        (WebCore::AXObjectCache::endCharacterOffsetOfWord):
+        (WebCore::AXObjectCache::previousWordStartCharacterOffset):
+        (WebCore::AXObjectCache::previousParagraphStartCharacterOffset):
+        (WebCore::AXObjectCache::previousSentenceStartCharacterOffset):
+        * accessibility/AXObjectCache.h:
+        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
+        (-[WebAccessibilityObjectWrapper doAXAttributedStringForTextMarkerRange:]):
+
 2016-02-19  Jer Noble  <jer.noble@apple.com>
 
         Allow CachedRawResource clients to opt out of caching on a per-response basis
index 8eeb732262ea87c7a322a074a036b8e01880fabb..d185be449652060b2a7318e91d7b85c60030c5a7 100644 (file)
@@ -1436,11 +1436,13 @@ CharacterOffset AXObjectCache::characterOffsetForTextMarkerData(TextMarkerData&
     return CharacterOffset(textMarkerData.node, textMarkerData.characterStartIndex, textMarkerData.characterOffset);
 }
 
-CharacterOffset AXObjectCache::traverseToOffsetInRange(RefPtr<Range>range, int offset, bool toNodeEnd, bool stayWithinRange)
+CharacterOffset AXObjectCache::traverseToOffsetInRange(RefPtr<Range>range, int offset, TraverseOption option, bool stayWithinRange)
 {
     if (!range)
         return CharacterOffset();
     
+    bool toNodeEnd = option & TraverseOptionToNodeEnd;
+    
     int offsetInCharacter = 0;
     int offsetSoFar = 0;
     int remaining = 0;
@@ -1464,7 +1466,7 @@ CharacterOffset AXObjectCache::traverseToOffsetInRange(RefPtr<Range>range, int o
             // Here when we don't have any character to move and we are going backwards, we traverse to the previous node.
             if (!lastLength && toNodeEnd && !stayWithinRange) {
                 if (Node* preNode = previousNode(currentNode))
-                    return traverseToOffsetInRange(rangeForNodeContents(preNode), offset, toNodeEnd);
+                    return traverseToOffsetInRange(rangeForNodeContents(preNode), offset, option);
                 return CharacterOffset();
             }
         }
@@ -1501,8 +1503,11 @@ CharacterOffset AXObjectCache::traverseToOffsetInRange(RefPtr<Range>range, int o
                     if (childNode && childNode->renderer() && childNode->renderer()->isBR()) {
                         currentNode = childNode;
                         hasReplacedNodeOrBR = true;
-                    } else if (currentNode != previousNode)
+                    } else if (currentNode != previousNode) {
+                        // We should set the currentNode to previous one in case this is the last iteration.
+                        currentNode = previousNode;
                         continue;
+                    }
                 }
             }
             offsetSoFar += currentLength;
@@ -1697,12 +1702,12 @@ CharacterOffset AXObjectCache::startOrEndCharacterOffsetForRange(RefPtr<Range> r
     Node* node = &copyRange->startContainer();
     if (node->offsetInCharacters()) {
         copyRange = Range::create(range->ownerDocument(), &range->startContainer(), range->startOffset(), &range->endContainer(), range->endOffset());
-        CharacterOffset nodeStartOffset = traverseToOffsetInRange(rangeForNodeContents(node), 0, false);
+        CharacterOffset nodeStartOffset = traverseToOffsetInRange(rangeForNodeContents(node), 0);
         offset = std::max(copyRange->startOffset() - nodeStartOffset.startIndex, 0);
         copyRange->setStart(node, nodeStartOffset.startIndex);
     }
     
-    return traverseToOffsetInRange(copyRange, offset, !isStart, stayWithinRange);
+    return traverseToOffsetInRange(copyRange, offset, isStart ? TraverseOptionDefault : TraverseOptionToNodeEnd, stayWithinRange);
 }
 
 void AXObjectCache::startOrEndTextMarkerDataForRange(TextMarkerData& textMarkerData, RefPtr<Range> range, bool isStart)
@@ -1750,13 +1755,13 @@ CharacterOffset AXObjectCache::characterOffsetForNodeAndOffset(Node& node, int o
 
     // Traverse the offset amount of characters forward and see if there's remaining offsets.
     // Keep traversing to the next node when there's remaining offsets.
-    CharacterOffset characterOffset = traverseToOffsetInRange(range, offset, toNodeEnd);
+    CharacterOffset characterOffset = traverseToOffsetInRange(range, offset, option);
     while (!characterOffset.isNull() && characterOffset.remaining() && !toNodeEnd) {
         domNode = nextNode(domNode);
         if (!domNode)
             return CharacterOffset();
         range = rangeForNodeContents(domNode);
-        characterOffset = traverseToOffsetInRange(range, characterOffset.remaining(), toNodeEnd);
+        characterOffset = traverseToOffsetInRange(range, characterOffset.remaining(), option);
     }
     
     return characterOffset;
@@ -1772,7 +1777,7 @@ void AXObjectCache::textMarkerDataForNextCharacterOffset(TextMarkerData& textMar
 {
     CharacterOffset next = characterOffset;
     do {
-        next = nextCharacterOffset(next);
+        next = nextCharacterOffset(next, false);
         textMarkerDataForCharacterOffset(textMarkerData, next);
     } while (textMarkerData.ignored);
 }
@@ -1781,7 +1786,7 @@ void AXObjectCache::textMarkerDataForPreviousCharacterOffset(TextMarkerData& tex
 {
     CharacterOffset previous = characterOffset;
     do {
-        previous = previousCharacterOffset(previous);
+        previous = previousCharacterOffset(previous, false);
         textMarkerDataForCharacterOffset(textMarkerData, previous);
     } while (textMarkerData.ignored);
 }
@@ -1818,8 +1823,11 @@ VisiblePosition AXObjectCache::visiblePositionFromCharacterOffset(const Characte
     // nextVisiblePosition means advancing one character. Use this to calculate the character offset.
     VisiblePositionRange vpRange = obj->visiblePositionRange();
     VisiblePosition start = vpRange.start;
+    
+    // Sometimes vpRange.start will be the previous node's end position and VisiblePosition will count the leading line break as 1 offset.
+    int characterCount = start.deepEquivalent().deprecatedNode() == characterOffset.node ? characterOffset.offset : characterOffset.offset + characterOffset.startIndex;
     VisiblePosition result = start;
-    for (int i = 0; i < characterOffset.offset; i++)
+    for (int i = 0; i < characterCount; i++)
         result = obj->nextVisiblePosition(result);
     
     return result;
@@ -1854,9 +1862,16 @@ CharacterOffset AXObjectCache::characterOffsetFromVisiblePosition(const VisibleP
         if (vpDeepPos.equals(previousVisiblePos.deepEquivalent()))
             break;
         characterOffset++;
+        
+        // When VisiblePostion moves to next node, it will count the leading line break as
+        // 1 offset, which we shouldn't include in CharacterOffset.
+        if (vpDeepPos.deprecatedNode() != previousVisiblePos.deepEquivalent().deprecatedNode()) {
+            if (vp.characterBefore() == '\n')
+                characterOffset--;
+        }
     }
     
-    return traverseToOffsetInRange(rangeForNodeContents(obj->node()), characterOffset, false);
+    return traverseToOffsetInRange(rangeForNodeContents(obj->node()), characterOffset);
 }
 
 AccessibilityObject* AXObjectCache::accessibilityObjectForTextMarkerData(TextMarkerData& textMarkerData)
@@ -1903,20 +1918,31 @@ void AXObjectCache::textMarkerDataForVisiblePosition(TextMarkerData& textMarkerD
     cache->setNodeInUse(domNode);
 }
 
-CharacterOffset AXObjectCache::nextCharacterOffset(const CharacterOffset& characterOffset, bool ignoreStart)
+CharacterOffset AXObjectCache::nextCharacterOffset(const CharacterOffset& characterOffset, bool ignoreNextNodeStart)
 {
     if (characterOffset.isNull())
         return CharacterOffset();
     
-    return characterOffsetForNodeAndOffset(*characterOffset.node, characterOffset.offset + 1, ignoreStart ? TraverseOptionDefault : TraverseOptionIncludeStart);
+    CharacterOffset next = characterOffsetForNodeAndOffset(*characterOffset.node, characterOffset.offset + 1);
+    
+    // To be consistent with VisiblePosition, we should consider the case that current node end to next node start counts 1 offset.
+    bool isReplacedOrBR = isReplacedNodeOrBR(characterOffset.node) || isReplacedNodeOrBR(next.node);
+    if (!ignoreNextNodeStart && !next.isNull() && !isReplacedOrBR && next.node != characterOffset.node)
+        next = characterOffsetForNodeAndOffset(*next.node, 0, TraverseOptionIncludeStart);
+    
+    return next;
 }
 
-CharacterOffset AXObjectCache::previousCharacterOffset(const CharacterOffset& characterOffset, bool ignoreStart)
+CharacterOffset AXObjectCache::previousCharacterOffset(const CharacterOffset& characterOffset, bool ignorePreviousNodeEnd)
 {
     if (characterOffset.isNull())
         return CharacterOffset();
     
-    return characterOffsetForNodeAndOffset(*characterOffset.node, characterOffset.offset - 1, ignoreStart ? TraverseOptionDefault : TraverseOptionIncludeStart);
+    // To be consistent with VisiblePosition, we should consider the case that current node start to previous node end counts 1 offset.
+    if (!ignorePreviousNodeEnd && !characterOffset.offset)
+        return characterOffsetForNodeAndOffset(*characterOffset.node, 0);
+    
+    return characterOffsetForNodeAndOffset(*characterOffset.node, characterOffset.offset - 1, TraverseOptionIncludeStart);
 }
 
 CharacterOffset AXObjectCache::startCharacterOffsetOfWord(const CharacterOffset& characterOffset, EWordSide side)
@@ -1949,9 +1975,13 @@ CharacterOffset AXObjectCache::endCharacterOffsetOfWord(const CharacterOffset& c
         if (c.isEqual(startOfParagraph))
             return c;
         
-        c = previousCharacterOffset(characterOffset, false);
+        c = previousCharacterOffset(characterOffset);
         if (c.isNull())
             return characterOffset;
+    } else {
+        CharacterOffset endOfParagraph = endCharacterOffsetOfParagraph(characterOffset);
+        if (characterOffset.isEqual(endOfParagraph))
+            return characterOffset;
     }
     
     return nextBoundary(c, endWordBoundary);
@@ -1962,7 +1992,7 @@ CharacterOffset AXObjectCache::previousWordStartCharacterOffset(const CharacterO
     if (characterOffset.isNull())
         return CharacterOffset();
     
-    CharacterOffset previousOffset = previousCharacterOffset(characterOffset, false);
+    CharacterOffset previousOffset = previousCharacterOffset(characterOffset);
     if (previousOffset.isNull())
         return CharacterOffset();
     
@@ -2207,11 +2237,11 @@ CharacterOffset AXObjectCache::nextParagraphEndCharacterOffset(const CharacterOf
 CharacterOffset AXObjectCache::previousParagraphStartCharacterOffset(const CharacterOffset& characterOffset)
 {
     // make sure we move off of a paragraph start
-    CharacterOffset previous = previousCharacterOffset(characterOffset, false);
+    CharacterOffset previous = previousCharacterOffset(characterOffset);
     
     // We should skip the preceding BR node.
     if (characterOffsetNodeIsBR(previous) && !characterOffsetNodeIsBR(characterOffset))
-        previous = previousCharacterOffset(previous, false);
+        previous = previousCharacterOffset(previous);
     
     return startCharacterOffsetOfParagraph(previous);
 }
@@ -2242,11 +2272,11 @@ CharacterOffset AXObjectCache::nextSentenceEndCharacterOffset(const CharacterOff
 CharacterOffset AXObjectCache::previousSentenceStartCharacterOffset(const CharacterOffset& characterOffset)
 {
     // Make sure we move off of a sentence start.
-    CharacterOffset previous = previousCharacterOffset(characterOffset, false);
+    CharacterOffset previous = previousCharacterOffset(characterOffset);
     
     // We should skip the preceding BR node.
-    if (!previous.isNull() && previous.node->hasTagName(brTag) && !characterOffset.node->hasTagName(brTag))
-        previous = previousCharacterOffset(previous, false);
+    if (characterOffsetNodeIsBR(previous) && !characterOffsetNodeIsBR(characterOffset))
+        previous = previousCharacterOffset(previous);
     
     return startCharacterOffsetOfSentence(previous);
 }
index 0e7661df47d87d92502cdb858e2ae82f58c4298f..c1dd2962eabe3285cc445e330ff949918e5d13f2 100644 (file)
@@ -196,8 +196,9 @@ public:
     void textMarkerDataForPreviousCharacterOffset(TextMarkerData&, const CharacterOffset&);
     VisiblePosition visiblePositionForTextMarkerData(TextMarkerData&);
     CharacterOffset characterOffsetForTextMarkerData(TextMarkerData&);
-    CharacterOffset nextCharacterOffset(const CharacterOffset&, bool ignoreStart = true);
-    CharacterOffset previousCharacterOffset(const CharacterOffset&, bool ignoreStart = true);
+    // Use ignoreNextNodeStart/ignorePreviousNodeEnd to determine the behavior when we are at node boundary. 
+    CharacterOffset nextCharacterOffset(const CharacterOffset&, bool ignoreNextNodeStart = true);
+    CharacterOffset previousCharacterOffset(const CharacterOffset&, bool ignorePreviousNodeEnd = true);
     void startOrEndTextMarkerDataForRange(TextMarkerData&, RefPtr<Range>, bool);
     AccessibilityObject* accessibilityObjectForTextMarkerData(TextMarkerData&);
     RefPtr<Range> rangeForUnorderedCharacterOffsets(const CharacterOffset&, const CharacterOffset&);
@@ -314,7 +315,7 @@ protected:
     enum TraverseOption { TraverseOptionDefault = 1 << 0, TraverseOptionToNodeEnd = 1 << 1, TraverseOptionIncludeStart = 1 << 2 };
     Node* nextNode(Node*) const;
     Node* previousNode(Node*) const;
-    CharacterOffset traverseToOffsetInRange(RefPtr<Range>, int, bool, bool stayWithinRange = false);
+    CharacterOffset traverseToOffsetInRange(RefPtr<Range>, int, TraverseOption = TraverseOptionDefault, bool stayWithinRange = false);
     VisiblePosition visiblePositionFromCharacterOffset(const CharacterOffset&);
     CharacterOffset characterOffsetFromVisiblePosition(const VisiblePosition&);
     void setTextMarkerDataWithCharacterOffset(TextMarkerData&, const CharacterOffset&);
index c395a7469659a7a61362638cb60c5f7bc2af367e..b8d0dcde2b6d84d60dea5b007756e5270fa5ec9c 100644 (file)
@@ -1293,19 +1293,9 @@ static NSString* nsStringForReplacedNode(Node* replacedNode)
     if (!m_object)
         return nil;
     
-    // extract the start and end VisiblePosition
-    VisiblePosition startVisiblePosition = visiblePositionForStartOfTextMarkerRange(m_object->axObjectCache(), textMarkerRange);
-    if (startVisiblePosition.isNull())
-        return nil;
-    
-    VisiblePosition endVisiblePosition = visiblePositionForEndOfTextMarkerRange(m_object->axObjectCache(), textMarkerRange);
-    if (endVisiblePosition.isNull())
-        return nil;
-    
-    VisiblePositionRange visiblePositionRange(startVisiblePosition, endVisiblePosition);
-    // iterate over the range to build the AX attributed string
+    RefPtr<Range> range = [self rangeForTextMarkerRange:textMarkerRange];
     NSMutableAttributedString* attrString = [[NSMutableAttributedString alloc] init];
-    TextIterator it(makeRange(startVisiblePosition, endVisiblePosition).get());
+    TextIterator it(range.get());
     while (!it.atEnd()) {
         // locate the node and starting offset for this range
         Node& node = it.range()->startContainer();