Changeset 201443 in webkit

Timestamp:

May 26, 2016 6:41:18 PM (8 years ago)

Author:

n_wang@apple.com

Message:

AX: Wrong CharacterOffset from VisiblePosition with composed characters
​https://bugs.webkit.org/show_bug.cgi?id=158138

Reviewed by Chris Fleizach.

Source/WebCore:

The conversion logic is not correct when the text node contains composed characters.
We should use VisiblePosition's offset directly for text nodes so we won't mess things up.

Test: accessibility/mac/character-offset-visible-position-conversion-with-emoji.html

• accessibility/AXObjectCache.cpp:

(WebCore::AXObjectCache::visiblePositionFromCharacterOffset):
(WebCore::AXObjectCache::characterOffsetFromVisiblePosition):

LayoutTests:

• accessibility/mac/character-offset-visible-position-conversion-with-emoji-expected.txt: Added.
• accessibility/mac/character-offset-visible-position-conversion-with-emoji.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/accessibility/mac/character-offset-visible-position-conversion-with-emoji-expected.txt (added)
• LayoutTests/accessibility/mac/character-offset-visible-position-conversion-with-emoji.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/accessibility/AXObjectCache.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-05-26  Nan Wang  <n_wang@apple.com>
+
+        AX: Wrong CharacterOffset from VisiblePosition with composed characters
+        https://bugs.webkit.org/show_bug.cgi?id=158138
+
+        Reviewed by Chris Fleizach.
+
+        * accessibility/mac/character-offset-visible-position-conversion-with-emoji-expected.txt: Added.
+        * accessibility/mac/character-offset-visible-position-conversion-with-emoji.html: Added.
+
 2016-05-26  Brent Fulgham  <bfulgham@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-05-26  Nan Wang  <n_wang@apple.com>
+
+        AX: Wrong CharacterOffset from VisiblePosition with composed characters
+        https://bugs.webkit.org/show_bug.cgi?id=158138
+
+        Reviewed by Chris Fleizach.
+
+        The conversion logic is not correct when the text node contains composed characters. 
+        We should use VisiblePosition's offset directly for text nodes so we won't mess things up.
+
+        Test: accessibility/mac/character-offset-visible-position-conversion-with-emoji.html
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::visiblePositionFromCharacterOffset):
+        (WebCore::AXObjectCache::characterOffsetFromVisiblePosition):
+
 2016-05-26  Darin Adler  <darin@apple.com>
 

trunk/Source/WebCore/accessibility/AXObjectCache.cpp

@@ -1953 +1953 @@
         return VisiblePosition();
     
-    RefPtr<AccessibilityObject> obj = this->getOrCreate(characterOffset.node);
-    if (!obj)
-        return VisiblePosition();
-    
-    // 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 < characterCount; i++)
-        result = obj->nextVisiblePosition(result);
-    
-    return result;
+    // Create a collapsed range and use that to form a VisiblePosition, so that the case with
+    // composed characters will be covered.
+    RefPtr<Range> range = rangeForUnorderedCharacterOffsets(characterOffset, characterOffset);
+    return VisiblePosition(range->startPosition());
 }
 
@@ -1978 +1967 @@
     Node* domNode = deepPos.deprecatedNode();
     ASSERT(domNode);
+    
+    if (domNode->offsetInCharacters())
+        return CharacterOffset(domNode, 0, deepPos.deprecatedEditingOffset(), 0);
     
     RefPtr<AccessibilityObject> obj = this->getOrCreate(domNode);