[PE] Improve SVG text selection for vertical text

We were previously always using the "left edge" of the fragment bounding box when computing the position within the fragment. This works poorly for vertical text. Use "top edge" for the vertical case instead. Bug: 829214 Change-Id: I52e0aeffa95d05b1c86cd6aeca5dde7a886db486 Reviewed-on: https://chromium-review.googlesource.com/997792 Commit-Queue: Fredrik Söderquist <fs@opera.com> Reviewed-by: Stephen Chenney <schenney@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#548722}

[PE] Improve SVG text selection for vertical text
We were previously always using the "left edge" of the fragment bounding box when computing the position within the fragment. This works poorly for vertical text. Use "top edge" for the vertical case instead. Bug: 829214 Change-Id: I52e0aeffa95d05b1c86cd6aeca5dde7a886db486 Reviewed-on: https://chromium-review.googlesource.com/997792 Commit-Queue: Fredrik Söderquist <fs@opera.com> Reviewed-by: Stephen Chenney <schenney@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#548722}
d8b3831c · Fredrik Söderquist · Commit Bot · 15d774ed · d8b3831c · d8b3831c
Commit d8b3831c authored Apr 06, 2018 by Fredrik Söderquist Committed by Commit Bot Apr 06, 2018
2 changed files
--- a/third_party/WebKit/LayoutTests/svg/text/select-text-vertical.html
+++ b/third_party/WebKit/LayoutTests/svg/text/select-text-vertical.html
+<!DOCTYPE html>
+<title>Selection of vertical text</title>
+<script src="../../resources/ahem.js"></script>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<style>body { margin: 0; }</style>
+<svg width="300" height="300">
+  <text id="text1" x="20" y="20" writing-mode="vertical-lr"
+        font-family="Ahem" font-size="20">MMMMM MMMMMMMM</text>
+</svg>
+<script>
+function selectText(start, end) {
+  let startPoint = { x: 10 + 10, y: 20 + start * 20 + 8 };
+  let endPoint = { x: 10 + 10, y: 20 + (end + 1) * 20 - 8 };
+
+  assert_exists(window, 'chrome');
+  assert_exists(window.chrome, 'gpuBenchmarking');
+  return new Promise(function(resolve, reject) {
+    chrome.gpuBenchmarking.pointerActionSequence(
+      [{
+        source: 'mouse',
+        actions: [
+          { name: 'pointerMove', x: startPoint.x, y: startPoint.y },
+          { name: 'pointerDown', x: startPoint.x, y: startPoint.y, button: 'left' },
+          { name: 'pointerMove', x: endPoint.x, y: endPoint.y },
+          { name: 'pointerUp', button: 'left' }
+        ]
+      }], resolve);
+  });
+}
+
+promise_test(function() {
+  return selectText(0, 0).then(() => {
+    var range = window.getSelection().getRangeAt(0);
+    assert_equals(range.startOffset, 0);
+    assert_equals(range.endOffset, 1);
+  });
+}, document.title+', start of text');
+
+promise_test(function() {
+  return selectText(4, 7).then(() => {
+    var range = window.getSelection().getRangeAt(0);
+    assert_equals(range.startOffset, 4);
+    assert_equals(range.endOffset, 8);
+  });
+}, document.title+', middle of text');
+
+promise_test(function() {
+  return selectText(13, 13).then(() => {
+    var range = window.getSelection().getRangeAt(0);
+    assert_equals(range.startOffset, 13);
+    assert_equals(range.endOffset, 14);
+  });
+}, document.title+', end of text');
+</script>
--- a/third_party/WebKit/Source/core/layout/svg/LayoutSVGInlineText.cpp
+++ b/third_party/WebKit/Source/core/layout/svg/LayoutSVGInlineText.cpp
@@ -173,7 +173,7 @@ PositionWithAffinity LayoutSVGInlineText::PositionForPoint(
  absolute_point.MoveBy(containing_block->Location());

  float closest_distance = std::numeric_limits<float>::max();
-  float closest_distance_position = 0;
+  float position_in_fragment = 0;
  const SVGTextFragment* closest_distance_fragment = nullptr;
  SVGInlineTextBox* closest_distance_box = nullptr;

@@ -193,7 +193,13 @@ PositionWithAffinity LayoutSVGInlineText::PositionForPoint(
        closest_distance = distance;
        closest_distance_box = text_box;
        closest_distance_fragment = &fragment;
-        closest_distance_position = fragment_rect.X();
+        // TODO(fs): This only works (reasonably) well for text with trivial
+        // transformations. For improved fidelity in the other cases we ought
+        // to apply the inverse transformation for the fragment and then map
+        // against the (untransformed) fragment rect.
+        position_in_fragment = fragment.is_vertical
+                                   ? absolute_point.Y() - fragment_rect.Y()
+                                   : absolute_point.X() - fragment_rect.X();
      }
    }
  }
@@ -202,8 +208,7 @@ PositionWithAffinity LayoutSVGInlineText::PositionForPoint(
    return CreatePositionWithAffinity(0);

  int offset = closest_distance_box->OffsetForPositionInFragment(
-      *closest_distance_fragment,
-      absolute_point.X() - closest_distance_position);
+      *closest_distance_fragment, position_in_fragment);
  return CreatePositionWithAffinity(offset + closest_distance_box->Start(),
                                    offset > 0
                                        ? TextAffinity::kUpstreamIfPossible