Fix ShapeResult::offsetForPosition for RTL scripts

CachingWordShapeIterator splits runs in the logical order, but ShapeResult::offsetForPosition() expected the input to be in the visual order. This patch fixes ShapeResult::offsetForPosition() to handle the list of ShapeResult from CachingWordShapeIterator in the logical order. This is a regression of "Change Font::offsetForPositionForComplexText to use CachingWordShapeIterator"[1]. Unfortunately tests in the CL did not catch this logic/visual inconsistency. The test was updated to include it. The test string has a diacritic mark at offset 1, and the test ensures offsetForPosition() dose not return 1. All platforms pass this criteria, however, only Mac test runner does not return offset 5 (Mac content shell is fine) so Mac -expeted is added separately. [1] https://codereview.chromium.org/1248453004/ BUG=518347 TEST=editing/selection/offset-from-point-complex-scripts.html Review URL: https://codereview.chromium.org/1314543003 git-svn-id: svn://svn.chromium.org/blink/trunk@201181 bbb929c8-8fbe-4397-9dbb-9b2b20218538

Fix ShapeResult::offsetForPosition for RTL scripts
CachingWordShapeIterator splits runs in the logical order, but ShapeResult::offsetForPosition() expected the input to be in the visual order. This patch fixes ShapeResult::offsetForPosition() to handle the list of ShapeResult from CachingWordShapeIterator in the logical order. This is a regression of "Change Font::offsetForPositionForComplexText to use CachingWordShapeIterator"[1]. Unfortunately tests in the CL did not catch this logic/visual inconsistency. The test was updated to include it. The test string has a diacritic mark at offset 1, and the test ensures offsetForPosition() dose not return 1. All platforms pass this criteria, however, only Mac test runner does not return offset 5 (Mac content shell is fine) so Mac -expeted is added separately. [1] https://codereview.chromium.org/1248453004/ BUG=518347 TEST=editing/selection/offset-from-point-complex-scripts.html Review URL: https://codereview.chromium.org/1314543003 git-svn-id: svn://svn.chromium.org/blink/trunk@201181 bbb929c8-8fbe-4397-9dbb-9b2b20218538
c42408e2 · kojii@chromium.org · bcd48bf1 · c42408e2 · c42408e2 · c42408e2
Commit c42408e2 authored Aug 26, 2015 by kojii@chromium.org
5 changed files
--- a/third_party/WebKit/LayoutTests/TestExpectations
+++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -303,6 +303,15 @@ crbug.com/475984 [ Mac Debug ] css2.1/t100801-c544-valgn-03-d-agi.html [ Failure
 # But consistently passes on release builds.
 crbug.com/518005 crbug.com/463358 [ Mac ] svg/W3C-SVG-1.1/struct-frag-02-t.svg [ Pass Failure ]
+crbug.com/518347 [ Mac ] editing/selection/vertical-rl-rtl-extend-line-backward-br.html [ NeedsRebaseline ]
+crbug.com/518347 [ Mac ] editing/selection/vertical-rl-rtl-extend-line-backward-p.html [ NeedsRebaseline ]
+crbug.com/518347 [ Mac ] editing/selection/vertical-rl-rtl-extend-line-forward-br.html [ NeedsRebaseline ]
+crbug.com/518347 [ SnowLeopard Lion Mavericks MountainLion Retina ] editing/selection/vertical-rl-rtl-extend-line-forward-p.html [ NeedsRebaseline ]
+crbug.com/518347 [ Mac ] fast/css/vertical-text-overflow-ellipsis-text-align-center.html [ NeedsRebaseline ]
+crbug.com/518347 [ Linux Mac ] fast/css/vertical-text-overflow-ellipsis-text-align-left.html [ NeedsRebaseline ]
+crbug.com/518347 [ Linux Mac XP Win7 ] fast/text/emphasis-ellipsis-complextext.html [ NeedsRebaseline ]
+crbug.com/518347 [ Linux ] fast/text/international/draw-complex-text-from-to.html [ NeedsRebaseline ]
 crbug.com/510337 cssom/cssvalue-comparison.html [ Slow ]
 crbug.com/510337 inspector/console/console-format.html [ Slow ]
 crbug.com/510337 inspector/elements/edit/edit-dom-actions.html [ Slow Timeout ]

--- a/third_party/WebKit/LayoutTests/editing/selection/offset-from-point-complex-scripts-expected.txt
+++ b/third_party/WebKit/LayoutTests/editing/selection/offset-from-point-complex-scripts-expected.txt
+0 5 4 3 2 6
--- a/third_party/WebKit/LayoutTests/editing/selection/offset-from-point-complex-scripts.html
+++ b/third_party/WebKit/LayoutTests/editing/selection/offset-from-point-complex-scripts.html
+<!DOCTYPE html>
+<meta charset="utf-8">
+<div id="container">
+    <p>مَ متن</p>
+</div>
+<div id=log></div>
+<script>
+testOffsetFromPoint(container.firstElementChild);
+function testOffsetFromPoint(element) {
+    var y = element.offsetTop + element.offsetHeight / 2;
+    var xmin = element.offsetLeft;
+    var xmax = xmin + element.offsetWidth;
+    var lastCharacterOffset = null;
+    var results = [];
+    for (var x = xmin - 1; x <= xmax + 1; ++x) {
+        var result = document.caretRangeFromPoint(x, y);
+        var characterOffset = result ? result.startOffset : null;
+        if (characterOffset === lastCharacterOffset)
+            continue;
+        results.push(characterOffset);
+        lastCharacterOffset = characterOffset;
+    }
+    var div = document.createElement("div");
+    div.innerText = results.join(" ");
+    log.appendChild(div);
+}
+if (window.testRunner) {
+    container.style.display = "none";
+    testRunner.dumpAsText();
+}
+</script>
--- a/third_party/WebKit/LayoutTests/platform/mac/editing/selection/offset-from-point-complex-scripts-expected.txt
+++ b/third_party/WebKit/LayoutTests/platform/mac/editing/selection/offset-from-point-complex-scripts-expected.txt
+0 4 3 2 6
--- a/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
+++ b/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
@@ -452,7 +452,8 @@ int ShapeResult::offsetForPosition(Vector<RefPtr<ShapeResult>>& results,
    unsigned totalOffset;
    if (run.rtl()) {
        totalOffset = run.length();
-        for (auto& wordResult : results) {
+        for (unsigned i = results.size(); i; --i) {
+            const RefPtr<ShapeResult>& wordResult = results[i - 1];
            if (!wordResult)
                continue;
            totalOffset -= wordResult->numCharacters();