<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[200258] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/200258">200258</a></dd>
<dt>Author</dt> <dd>n_wang@apple.com</dd>
<dt>Date</dt> <dd>2016-04-29 13:05:07 -0700 (Fri, 29 Apr 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>AX: CharacterOffset not working correctly with composed characters and collapsed white spaces
https://bugs.webkit.org/show_bug.cgi?id=157190
Reviewed by Chris Fleizach.
Source/WebCore:
When navigating emoji, next/previous text marker call is only moving by one character. Fixed it by
using the helper function in Position to get the real character count for the composed character sequence.
Also there's another issue with collapsed white spaces, TextIterator emits only one space. So we have to
use the actual space length to create the CharacterOffset in order to generate valid Range object from it.
New test cases in accessibility/text-marker/text-marker-previous-next.html.
* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::traverseToOffsetInRange):
(WebCore::AXObjectCache::textMarkerDataForNextCharacterOffset):
(WebCore::AXObjectCache::textMarkerDataForPreviousCharacterOffset):
(WebCore::AXObjectCache::nextNode):
(WebCore::AXObjectCache::characterOffsetFromVisiblePosition):
(WebCore::AXObjectCache::nextCharacterOffset):
(WebCore::AXObjectCache::previousCharacterOffset):
(WebCore::AXObjectCache::startCharacterOffsetOfWord):
LayoutTests:
* accessibility/mac/text-marker-word-nav.html:
* accessibility/text-marker/text-marker-previous-next-expected.txt:
* accessibility/text-marker/text-marker-previous-next.html:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsaccessibilitymactextmarkerwordnavhtml">trunk/LayoutTests/accessibility/mac/text-marker-word-nav.html</a></li>
<li><a href="#trunkLayoutTestsaccessibilitytextmarkertextmarkerpreviousnextexpectedtxt">trunk/LayoutTests/accessibility/text-marker/text-marker-previous-next-expected.txt</a></li>
<li><a href="#trunkLayoutTestsaccessibilitytextmarkertextmarkerpreviousnexthtml">trunk/LayoutTests/accessibility/text-marker/text-marker-previous-next.html</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreaccessibilityAXObjectCachecpp">trunk/Source/WebCore/accessibility/AXObjectCache.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (200257 => 200258)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2016-04-29 20:01:42 UTC (rev 200257)
+++ trunk/LayoutTests/ChangeLog 2016-04-29 20:05:07 UTC (rev 200258)
</span><span class="lines">@@ -1,3 +1,14 @@
</span><ins>+2016-04-29 Nan Wang <n_wang@apple.com>
+
+ AX: CharacterOffset not working correctly with composed characters and collapsed white spaces
+ https://bugs.webkit.org/show_bug.cgi?id=157190
+
+ Reviewed by Chris Fleizach.
+
+ * accessibility/mac/text-marker-word-nav.html:
+ * accessibility/text-marker/text-marker-previous-next-expected.txt:
+ * accessibility/text-marker/text-marker-previous-next.html:
+
</ins><span class="cx"> 2016-04-29 Ryan Haddad <ryanhaddad@apple.com>
</span><span class="cx">
</span><span class="cx"> Marking fast/ruby/ruby-expansion-cjk.html and fast/ruby/ruby-expansion-cjk-4.html as flaky on Mac
</span></span></pre></div>
<a id="trunkLayoutTestsaccessibilitymactextmarkerwordnavhtml"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/accessibility/mac/text-marker-word-nav.html (200257 => 200258)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/accessibility/mac/text-marker-word-nav.html 2016-04-29 20:01:42 UTC (rev 200257)
+++ trunk/LayoutTests/accessibility/mac/text-marker-word-nav.html 2016-04-29 20:05:07 UTC (rev 200258)
</span><span class="lines">@@ -67,7 +67,7 @@
</span><span class="cx">
</span><span class="cx"> // Check the case with span
</span><span class="cx"> // At "T" in "Thisis", should return the word as "Thisislongword".
</span><del>- currentMarker = advanceAndVerify(currentMarker, 2, text);
</del><ins>+ currentMarker = advanceAndVerify(currentMarker, 1, text);
</ins><span class="cx"> // At " " before "I", the word should be "I'll".
</span><span class="cx"> currentMarker = advanceAndVerify(currentMarker, 14, text);
</span><span class="cx"> // At " " before "try", the word should excludes "."
</span></span></pre></div>
<a id="trunkLayoutTestsaccessibilitytextmarkertextmarkerpreviousnextexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/accessibility/text-marker/text-marker-previous-next-expected.txt (200257 => 200258)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/accessibility/text-marker/text-marker-previous-next-expected.txt 2016-04-29 20:01:42 UTC (rev 200257)
+++ trunk/LayoutTests/accessibility/text-marker/text-marker-previous-next-expected.txt 2016-04-29 20:05:07 UTC (rev 200258)
</span><span class="lines">@@ -5,6 +5,10 @@
</span><span class="cx">
</span><span class="cx"> can't select
</span><span class="cx"> abc de f
</span><ins>+😃😏
+
+a b
+
</ins><span class="cx"> This tests the next/previous text marker functions are implemented correctly.
</span><span class="cx">
</span><span class="cx"> On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
</span><span class="lines">@@ -28,6 +32,15 @@
</span><span class="cx"> PASS text2.accessibilityElementForTextMarker(currentMarker).isEqual(text2.childAtIndex(2)) is true
</span><span class="cx"> PASS text.stringForTextMarkerRange(markerRange) is 'f'
</span><span class="cx"> PASS text.stringForTextMarkerRange(markerRange) is 'a'
</span><ins>+PASS text.textMarkerRangeLength(emojiTextMarkerRange) is 4
+PASS text.stringForTextMarkerRange(text.textMarkerRangeForMarkers(result.previous, result.current)) is '😏'
+PASS text.stringForTextMarkerRange(text.textMarkerRangeForMarkers(result.previous, result.current)) is '😃'
+PASS text.textMarkerRangeLength(collapsedWhitespaceMarkerRange) is 3
+PASS text.stringForTextMarkerRange(text.textMarkerRangeForMarkers(result.previous, result.current)) is 'a'
+PASS text.stringForTextMarkerRange(text.textMarkerRangeForMarkers(result.previous, result.current)) is ' '
+PASS text.stringForTextMarkerRange(text.textMarkerRangeForMarkers(result.previous, result.current)) is 'b'
+PASS text.stringForTextMarkerRange(text.textMarkerRangeForMarkers(result.previous, result.current)) is ' '
+PASS text.stringForTextMarkerRange(text.textMarkerRangeForMarkers(result.previous, result.current)) is 'a'
</ins><span class="cx"> PASS successfullyParsed is true
</span><span class="cx">
</span><span class="cx"> TEST COMPLETE
</span></span></pre></div>
<a id="trunkLayoutTestsaccessibilitytextmarkertextmarkerpreviousnexthtml"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/accessibility/text-marker/text-marker-previous-next.html (200257 => 200258)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/accessibility/text-marker/text-marker-previous-next.html 2016-04-29 20:01:42 UTC (rev 200257)
+++ trunk/LayoutTests/accessibility/text-marker/text-marker-previous-next.html 2016-04-29 20:05:07 UTC (rev 200258)
</span><span class="lines">@@ -2,6 +2,7 @@
</span><span class="cx"> <html>
</span><span class="cx"> <head>
</span><span class="cx"> <script src="../../resources/js-test-pre.js"></script>
</span><ins>+<meta charset="UTF-16">
</ins><span class="cx"> </head>
</span><span class="cx"> <style>
</span><span class="cx"> .userselect { user-select: none; -webkit-user-select: none; }
</span><span class="lines">@@ -26,6 +27,10 @@
</span><span class="cx"> f
</span><span class="cx"> </div>
</span><span class="cx">
</span><ins>+<p id="text5">😃😏<p>
+
+<p id="text6">a b</p>
+
</ins><span class="cx"> <p id="description"></p>
</span><span class="cx"> <div id="console"></div>
</span><span class="cx">
</span><span class="lines">@@ -71,8 +76,8 @@
</span><span class="cx"> markerRange = text.textMarkerRangeForMarkers(previousMarker, currentMarker);
</span><span class="cx"> shouldBe("text.stringForTextMarkerRange(markerRange)", "newline");
</span><span class="cx">
</span><del>- // Traverse backwards two more character, it will land at the last character of "text".
- result = backwards(2, previousMarker, currentMarker, text);
</del><ins>+ // Traverse backwards one more character, it will land at the last character of "text".
+ result = backwards(1, previousMarker, currentMarker, text);
</ins><span class="cx"> previousMarker = result.previous;
</span><span class="cx"> currentMarker = result.current;
</span><span class="cx"> markerRange = text.textMarkerRangeForMarkers(previousMarker, currentMarker);
</span><span class="lines">@@ -93,8 +98,8 @@
</span><span class="cx"> markerRange = text2.textMarkerRangeForMarkers(previousMarker, currentMarker);
</span><span class="cx"> shouldBe("text2.stringForTextMarkerRange(markerRange)", "'d'");
</span><span class="cx">
</span><del>- // Traverse backwards 8 characters, it will land at the last character of "text1".
- result = backwards(8, previousMarker, currentMarker, text2);
</del><ins>+ // Traverse backwards 6 characters, it will land at the last character of "text1".
+ result = backwards(6, previousMarker, currentMarker, text2);
</ins><span class="cx"> previousMarker = result.previous;
</span><span class="cx"> currentMarker = result.current;
</span><span class="cx"> markerRange = text2.textMarkerRangeForMarkers(previousMarker, currentMarker);
</span><span class="lines">@@ -163,6 +168,33 @@
</span><span class="cx"> markerRange = text.textMarkerRangeForMarkers(startMarker, currentMarker)
</span><span class="cx"> shouldBe("text.stringForTextMarkerRange(markerRange)", "'a'");
</span><span class="cx">
</span><ins>+ // Test case with emoji.
+ text = accessibilityController.accessibleElementById("text5");
+ var emojiTextMarkerRange = text.textMarkerRangeForElement(text);
+ shouldBe("text.textMarkerRangeLength(emojiTextMarkerRange)", "4");
+ // Make sure navigating next/previous text marker is by emoji.
+ startMarker = text.startTextMarkerForTextMarkerRange(emojiTextMarkerRange);
+ result = forward(2, previousMarker, startMarker, text);
+ shouldBe("text.stringForTextMarkerRange(text.textMarkerRangeForMarkers(result.previous, result.current))", "'😏'");
+ result = backwards(1, result.previous, result.current, text);
+ shouldBe("text.stringForTextMarkerRange(text.textMarkerRangeForMarkers(result.previous, result.current))", "'😃'");
+
+ // Test case with collapsed whitespace.
+ text = accessibilityController.accessibleElementById("text6");
+ var collapsedWhitespaceMarkerRange = text.textMarkerRangeForElement(text);
+ shouldBe("text.textMarkerRangeLength(collapsedWhitespaceMarkerRange)", "3");
+ startMarker = text.startTextMarkerForTextMarkerRange(collapsedWhitespaceMarkerRange);
+ result = forward(1, previousMarker, startMarker, text);
+ shouldBe("text.stringForTextMarkerRange(text.textMarkerRangeForMarkers(result.previous, result.current))", "'a'");
+ result = forward(1, result.previous, result.current, text);
+ shouldBe("text.stringForTextMarkerRange(text.textMarkerRangeForMarkers(result.previous, result.current))", "' '");
+ result = forward(1, result.previous, result.current, text);
+ shouldBe("text.stringForTextMarkerRange(text.textMarkerRangeForMarkers(result.previous, result.current))", "'b'");
+ result = backwards(1, result.previous, result.current, text);
+ shouldBe("text.stringForTextMarkerRange(text.textMarkerRangeForMarkers(result.previous, result.current))", "' '");
+ result = backwards(1, result.previous, result.current, text);
+ shouldBe("text.stringForTextMarkerRange(text.textMarkerRangeForMarkers(result.previous, result.current))", "'a'");
+
</ins><span class="cx"> function forward(count, previousMarker, currentMarker, obj) {
</span><span class="cx"> for (var i = 0; i < count; i++) {
</span><span class="cx"> previousMarker = currentMarker;
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (200257 => 200258)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-04-29 20:01:42 UTC (rev 200257)
+++ trunk/Source/WebCore/ChangeLog 2016-04-29 20:05:07 UTC (rev 200258)
</span><span class="lines">@@ -1,3 +1,27 @@
</span><ins>+2016-04-29 Nan Wang <n_wang@apple.com>
+
+ AX: CharacterOffset not working correctly with composed characters and collapsed white spaces
+ https://bugs.webkit.org/show_bug.cgi?id=157190
+
+ Reviewed by Chris Fleizach.
+
+ When navigating emoji, next/previous text marker call is only moving by one character. Fixed it by
+ using the helper function in Position to get the real character count for the composed character sequence.
+ Also there's another issue with collapsed white spaces, TextIterator emits only one space. So we have to
+ use the actual space length to create the CharacterOffset in order to generate valid Range object from it.
+
+ New test cases in accessibility/text-marker/text-marker-previous-next.html.
+
+ * accessibility/AXObjectCache.cpp:
+ (WebCore::AXObjectCache::traverseToOffsetInRange):
+ (WebCore::AXObjectCache::textMarkerDataForNextCharacterOffset):
+ (WebCore::AXObjectCache::textMarkerDataForPreviousCharacterOffset):
+ (WebCore::AXObjectCache::nextNode):
+ (WebCore::AXObjectCache::characterOffsetFromVisiblePosition):
+ (WebCore::AXObjectCache::nextCharacterOffset):
+ (WebCore::AXObjectCache::previousCharacterOffset):
+ (WebCore::AXObjectCache::startCharacterOffsetOfWord):
+
</ins><span class="cx"> 2016-04-28 Jer Noble <jer.noble@apple.com>
</span><span class="cx">
</span><span class="cx"> WebPlaybackControlsManager should not be owned by the WebPlaybackSessionInterfaceMac.
</span></span></pre></div>
<a id="trunkSourceWebCoreaccessibilityAXObjectCachecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (200257 => 200258)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2016-04-29 20:01:42 UTC (rev 200257)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2016-04-29 20:05:07 UTC (rev 200258)
</span><span class="lines">@@ -1479,7 +1479,7 @@
</span><span class="cx"> bool toNodeEnd = option & TraverseOptionToNodeEnd;
</span><span class="cx">
</span><span class="cx"> int offsetInCharacter = 0;
</span><del>- int offsetSoFar = 0;
</del><ins>+ int cumulativeOffset = 0;
</ins><span class="cx"> int remaining = 0;
</span><span class="cx"> int lastLength = 0;
</span><span class="cx"> Node* currentNode = nullptr;
</span><span class="lines">@@ -1494,8 +1494,8 @@
</span><span class="cx"> lastStartOffset = range->startOffset();
</span><span class="cx"> if (offset > 0 || toNodeEnd) {
</span><span class="cx"> if (AccessibilityObject::replacedNodeNeedsCharacter(currentNode) || (currentNode->renderer() && currentNode->renderer()->isBR()))
</span><del>- offsetSoFar++;
- lastLength = offsetSoFar;
</del><ins>+ cumulativeOffset++;
+ lastLength = cumulativeOffset;
</ins><span class="cx">
</span><span class="cx"> // When going backwards, stayWithinRange is false.
</span><span class="cx"> // Here when we don't have any character to move and we are going backwards, we traverse to the previous node.
</span><span class="lines">@@ -1510,19 +1510,27 @@
</span><span class="cx"> // Sometimes text contents in a node are splitted into several iterations, so that iterator.range()->startOffset()
</span><span class="cx"> // might not be the correct character count. Here we use a previousNode object to keep track of that.
</span><span class="cx"> Node* previousNode = nullptr;
</span><ins>+ // When text node has collapsed whitespaces, we need to treat it differently since text iterator
+ // will omit the collapsed spaces and make the offset inaccurate.
+ Node* collapsedWhitespaceNode = nullptr;
</ins><span class="cx"> for (; !iterator.atEnd(); iterator.advance()) {
</span><span class="cx"> int currentLength = iterator.text().length();
</span><span class="cx"> bool hasReplacedNodeOrBR = false;
</span><span class="cx">
</span><span class="cx"> Node& node = iterator.range()->startContainer();
</span><span class="cx"> currentNode = &node;
</span><ins>+
+ // The offset of node with collapsed whitespaces has been calcualted in the first iteration.
+ if (currentNode == collapsedWhitespaceNode)
+ continue;
+
</ins><span class="cx"> // When currentLength == 0, we check if there's any replaced node.
</span><span class="cx"> // If not, we skip the node with no length.
</span><span class="cx"> if (!currentLength) {
</span><span class="cx"> int subOffset = iterator.range()->startOffset();
</span><span class="cx"> Node* childNode = node.traverseToChildAt(subOffset);
</span><span class="cx"> if (AccessibilityObject::replacedNodeNeedsCharacter(childNode)) {
</span><del>- offsetSoFar++;
</del><ins>+ cumulativeOffset++;
</ins><span class="cx"> currentLength++;
</span><span class="cx"> currentNode = childNode;
</span><span class="cx"> hasReplacedNodeOrBR = true;
</span><span class="lines">@@ -1545,8 +1553,30 @@
</span><span class="cx"> continue;
</span><span class="cx"> }
</span><span class="cx"> }
</span><ins>+ } else if (currentNode->isTextNode() && currentNode->renderer()) {
+ // When there's collapsed whitespace, the text iterator will only count those spaces as one single space.
+ // Here we use the RenderText to get the actual length.
+ RenderText* renderedText = downcast<RenderText>(currentNode->renderer());
+ int currentStartOffset = iterator.range()->startOffset();
+ if (renderedText->style().isCollapsibleWhiteSpace(iterator.text()[currentLength - 1]) && currentLength + currentStartOffset != renderedText->caretMaxOffset()) {
+ int appendLength = (&range->endContainer() == currentNode ? range->endOffset() : (int)renderedText->text()->length()) - currentStartOffset;
+ lastStartOffset = currentStartOffset;
+ cumulativeOffset += appendLength;
+ lastLength = appendLength;
+
+ // Break early if we have advanced enough characters.
+ if (!toNodeEnd && cumulativeOffset >= offset) {
+ offsetInCharacter = offset - (cumulativeOffset - lastLength);
+ finished = true;
+ break;
+ }
+
+ collapsedWhitespaceNode = currentNode;
+ continue;
+ }
+
</ins><span class="cx"> }
</span><del>- offsetSoFar += currentLength;
</del><ins>+ cumulativeOffset += currentLength;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if (currentNode == previousNode)
</span><span class="lines">@@ -1557,8 +1587,8 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // Break early if we have advanced enough characters.
</span><del>- if (!toNodeEnd && offsetSoFar >= offset) {
- offsetInCharacter = offset - (offsetSoFar - lastLength);
</del><ins>+ if (!toNodeEnd && cumulativeOffset >= offset) {
+ offsetInCharacter = offset - (cumulativeOffset - lastLength);
</ins><span class="cx"> finished = true;
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="lines">@@ -1568,7 +1598,7 @@
</span><span class="cx"> if (!finished) {
</span><span class="cx"> offsetInCharacter = lastLength;
</span><span class="cx"> if (!toNodeEnd)
</span><del>- remaining = offset - offsetSoFar;
</del><ins>+ remaining = offset - cumulativeOffset;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> return CharacterOffset(currentNode, lastStartOffset, offsetInCharacter, remaining);
</span><span class="lines">@@ -1850,22 +1880,36 @@
</span><span class="cx"> {
</span><span class="cx"> CharacterOffset next = characterOffset;
</span><span class="cx"> CharacterOffset previous = characterOffset;
</span><ins>+ bool shouldContinue;
</ins><span class="cx"> do {
</span><ins>+ shouldContinue = false;
</ins><span class="cx"> next = nextCharacterOffset(next, false);
</span><span class="cx"> if (shouldSkipBoundary(previous, next))
</span><span class="cx"> next = nextCharacterOffset(next, false);
</span><span class="cx"> textMarkerDataForCharacterOffset(textMarkerData, next);
</span><ins>+
+ // We should skip next CharactetOffset if it's visually the same.
+ if (!lengthForRange(rangeForUnorderedCharacterOffsets(previous, next).get()))
+ shouldContinue = true;
</ins><span class="cx"> previous = next;
</span><del>- } while (textMarkerData.ignored);
</del><ins>+ } while (textMarkerData.ignored || shouldContinue);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void AXObjectCache::textMarkerDataForPreviousCharacterOffset(TextMarkerData& textMarkerData, const CharacterOffset& characterOffset)
</span><span class="cx"> {
</span><span class="cx"> CharacterOffset previous = characterOffset;
</span><ins>+ CharacterOffset next = characterOffset;
+ bool shouldContinue;
</ins><span class="cx"> do {
</span><ins>+ shouldContinue = false;
</ins><span class="cx"> previous = previousCharacterOffset(previous, false);
</span><span class="cx"> textMarkerDataForCharacterOffset(textMarkerData, previous);
</span><del>- } while (textMarkerData.ignored);
</del><ins>+
+ // We should skip previous CharactetOffset if it's visually the same.
+ if (!lengthForRange(rangeForUnorderedCharacterOffsets(previous, next).get()))
+ shouldContinue = true;
+ next = previous;
+ } while (textMarkerData.ignored || shouldContinue);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> Node* AXObjectCache::nextNode(Node* node) const
</span><span class="lines">@@ -1924,27 +1968,32 @@
</span><span class="cx"> return CharacterOffset();
</span><span class="cx">
</span><span class="cx"> // Use nextVisiblePosition to calculate how many characters we need to traverse to the current position.
</span><del>- VisiblePositionRange vpRange = obj->visiblePositionRange();
- VisiblePosition vp = vpRange.start;
</del><ins>+ VisiblePositionRange visiblePositionRange = obj->visiblePositionRange();
+ VisiblePosition visiblePosition = visiblePositionRange.start;
</ins><span class="cx"> int characterOffset = 0;
</span><del>- Position vpDeepPos = vp.deepEquivalent();
</del><ins>+ Position currentPosition = visiblePosition.deepEquivalent();
</ins><span class="cx">
</span><span class="cx"> VisiblePosition previousVisiblePos;
</span><del>- while (!vpDeepPos.isNull() && !deepPos.equals(vpDeepPos)) {
- previousVisiblePos = vp;
- vp = obj->nextVisiblePosition(vp);
- vpDeepPos = vp.deepEquivalent();
</del><ins>+ while (!currentPosition.isNull() && !deepPos.equals(currentPosition)) {
+ previousVisiblePos = visiblePosition;
+ visiblePosition = obj->nextVisiblePosition(visiblePosition);
+ currentPosition = visiblePosition.deepEquivalent();
+ Position previousPosition = previousVisiblePos.deepEquivalent();
</ins><span class="cx"> // Sometimes nextVisiblePosition will give the same VisiblePostion,
</span><span class="cx"> // we break here to avoid infinite loop.
</span><del>- if (vpDeepPos.equals(previousVisiblePos.deepEquivalent()))
</del><ins>+ if (currentPosition.equals(previousPosition))
</ins><span class="cx"> break;
</span><span class="cx"> characterOffset++;
</span><span class="cx">
</span><span class="cx"> // When VisiblePostion moves to next node, it will count the leading line break as
</span><span class="cx"> // 1 offset, which we shouldn't include in CharacterOffset.
</span><del>- if (vpDeepPos.deprecatedNode() != previousVisiblePos.deepEquivalent().deprecatedNode()) {
- if (vp.characterBefore() == '\n')
</del><ins>+ if (currentPosition.deprecatedNode() != previousPosition.deprecatedNode()) {
+ if (visiblePosition.characterBefore() == '\n')
</ins><span class="cx"> characterOffset--;
</span><ins>+ } else {
+ // Sometimes VisiblePosition will move multiple characters, like emoji.
+ if (currentPosition.deprecatedNode()->offsetInCharacters())
+ characterOffset += currentPosition.offsetInContainerNode() - previousPosition.offsetInContainerNode() - 1;
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -2006,7 +2055,9 @@
</span><span class="cx"> if (characterOffset.isNull())
</span><span class="cx"> return CharacterOffset();
</span><span class="cx">
</span><del>- CharacterOffset next = characterOffsetForNodeAndOffset(*characterOffset.node, characterOffset.offset + 1);
</del><ins>+ // We don't always move one 'character' at a time since there might be composed characters.
+ int nextOffset = Position::uncheckedNextOffset(characterOffset.node, characterOffset.offset);
+ CharacterOffset next = characterOffsetForNodeAndOffset(*characterOffset.node, nextOffset);
</ins><span class="cx">
</span><span class="cx"> // To be consistent with VisiblePosition, we should consider the case that current node end to next node start counts 1 offset.
</span><span class="cx"> bool isReplacedOrBR = isReplacedNodeOrBR(characterOffset.node) || isReplacedNodeOrBR(next.node);
</span><span class="lines">@@ -2025,7 +2076,9 @@
</span><span class="cx"> if (!ignorePreviousNodeEnd && !characterOffset.offset)
</span><span class="cx"> return characterOffsetForNodeAndOffset(*characterOffset.node, 0);
</span><span class="cx">
</span><del>- return characterOffsetForNodeAndOffset(*characterOffset.node, characterOffset.offset - 1, TraverseOptionIncludeStart);
</del><ins>+ // We don't always move one 'character' a time since there might be composed characters.
+ int previousOffset = Position::uncheckedPreviousOffset(characterOffset.node, characterOffset.offset);
+ return characterOffsetForNodeAndOffset(*characterOffset.node, previousOffset, TraverseOptionIncludeStart);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> CharacterOffset AXObjectCache::startCharacterOffsetOfWord(const CharacterOffset& characterOffset, EWordSide side)
</span></span></pre>
</div>
</div>
</body>
</html>