Fix ScrollFocusedEditableIntoView for selections

This method is used to scroll and zoom an editable element into view and to a legible scale. This generally happens on Android where the on-screen keyboard appears and the input box is zoomed into the viewport. The linked bug occurs because we try to zoom the input box while it has a selection (e.g Ctrl-A the text) rather than a blinking caret. This method would then receive an empty rect for the caret_rect which would cause us to zoom into the maximum possible scale. The fix here is to fallback to the selection rect if there is no carret. Bug: 968638 Change-Id: I980183bf9843192e694579cf2af4af816b78a090 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1642248Reviewed-by: Stefan Zager <szager@chromium.org> Commit-Queue: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#666493}

Fix ScrollFocusedEditableIntoView for selections
This method is used to scroll and zoom an editable element into view and to a legible scale. This generally happens on Android where the on-screen keyboard appears and the input box is zoomed into the viewport. The linked bug occurs because we try to zoom the input box while it has a selection (e.g Ctrl-A the text) rather than a blinking caret. This method would then receive an empty rect for the caret_rect which would cause us to zoom into the maximum possible scale. The fix here is to fallback to the selection rect if there is no carret. Bug: 968638 Change-Id: I980183bf9843192e694579cf2af4af816b78a090 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1642248Reviewed-by: Stefan Zager <szager@chromium.org> Commit-Queue: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#666493}
c5b24f8c · David Bokan · Commit Bot · 88a133da · c5b24f8c · c5b24f8c
Commit c5b24f8c authored Jun 05, 2019 by David Bokan Committed by Commit Bot Jun 05, 2019
3 changed files
--- a/third_party/blink/renderer/core/editing/frame_selection.h
+++ b/third_party/blink/renderer/core/editing/frame_selection.h
@@ -202,6 +202,10 @@ class CORE_EXPORT FrameSelection final
  // Note: this updates styles and layout, use cautiously.
  bool ComputeAbsoluteBounds(IntRect& anchor, IntRect& focus) const;

+  // Computes the rect we should use when scrolling/zooming a selection into
+  // view.
+  IntRect ComputeRectToScroll(RevealExtentOption);
+
  void DidChangeFocus();

  SelectionInDOMTree GetSelectionInDOMTree() const;
@@ -300,8 +304,6 @@ class CORE_EXPORT FrameSelection final

  GranularityStrategy* GetGranularityStrategy();

-  IntRect ComputeRectToScroll(RevealExtentOption);
-
  void MoveRangeSelectionInternal(const SelectionInDOMTree&, TextGranularity);

  // Implementation of |SynchronousMutationObserver| member functions.

--- a/third_party/blink/renderer/core/exported/web_frame_test.cc
+++ b/third_party/blink/renderer/core/exported/web_frame_test.cc
@@ -120,6 +120,7 @@
 #include "third_party/blink/renderer/core/fullscreen/fullscreen.h"
 #include "third_party/blink/renderer/core/geometry/dom_rect.h"
 #include "third_party/blink/renderer/core/html/forms/html_form_element.h"
+#include "third_party/blink/renderer/core/html/forms/html_input_element.h"
 #include "third_party/blink/renderer/core/html/html_body_element.h"
 #include "third_party/blink/renderer/core/html/html_iframe_element.h"
 #include "third_party/blink/renderer/core/html/image_document.h"
@@ -11743,6 +11744,58 @@ TEST_F(WebFrameSimTest, ScrollFocusedIntoViewClipped) {
  EXPECT_GT(clip->scrollTop(), 0);
 }

+//  This test ensures that we scroll to the correct scale when the focused
+//  element has a selection rather than a carret.
+TEST_F(WebFrameSimTest, ScrollFocusedSelectionIntoView) {
+  UseAndroidSettings();
+  WebView().MainFrameWidget()->Resize(WebSize(400, 600));
+  WebView().EnableFakePageScaleAnimationForTesting(true);
+  WebView().GetPage()->GetSettings().SetTextAutosizingEnabled(false);
+
+  SimRequest request("https://example.com/test.html", "text/html");
+  LoadURL("https://example.com/test.html");
+  request.Complete(R"HTML(
+      <!DOCTYPE html>
+      <style>
+        ::-webkit-scrollbar {
+          width: 0px;
+          height: 0px;
+        }
+        body, html {
+          margin: 0px;
+          width: 100%;
+          height: 100%;
+        }
+        input {
+          padding: 0;
+          width: 100px;
+          height: 20px;
+        }
+      </style>
+      <input type="text" id="target" value="test">
+  )HTML");
+
+  Compositor().BeginFrame();
+  WebView().AdvanceFocus(false);
+
+  HTMLInputElement* input =
+      ToHTMLInputElement(GetDocument().getElementById("target"));
+  input->select();
+
+  // Simulate the keyboard being shown and resizing the widget. Cause a scroll
+  // into view after.
+  ASSERT_EQ(WebView().FakePageScaleAnimationPageScaleForTesting(), 0.f);
+  WebFrameWidget* widget = WebView().MainFrameImpl()->FrameWidgetImpl();
+  widget->ScrollFocusedEditableElementIntoView();
+
+  // Make sure zoomed in but only up to a legible scale. The bounds are
+  // arbitrary and fuzzy since we don't specifically care to constrain the
+  // amount of zooming (that should be tested elsewhere), we just care that it
+  // zooms but not off to infinity.
+  EXPECT_GT(WebView().FakePageScaleAnimationPageScaleForTesting(), .75f);
+  EXPECT_LT(WebView().FakePageScaleAnimationPageScaleForTesting(), 2.f);
+}
+
 TEST_F(WebFrameSimTest, DoubleTapZoomWhileScrolled) {
  UseAndroidSettings();
  WebView().MainFrameWidget()->Resize(WebSize(490, 500));

--- a/third_party/blink/renderer/core/exported/web_view_impl.cc
+++ b/third_party/blink/renderer/core/exported/web_view_impl.cc
@@ -2160,7 +2160,7 @@ bool WebViewImpl::ScrollFocusedEditableElementIntoView() {
              element->GetDocument()
                  .GetFrame()
                  ->Selection()
-                  .AbsoluteCaretBounds())),
+                  .ComputeRectToScroll(kDoNotRevealExtent))),
      ShouldZoomToLegibleScale(*element));

  return true;