[go: up one dir, main page]

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.

(cherry picked from commit c5b24f8cb108411a0588b189742cee9b939a7675)

Bug: 968638
Change-Id: I980183bf9843192e694579cf2af4af816b78a090
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1642248
Reviewed-by: Stefan Zager <szager@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#666493}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1652580
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#212}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/third_party/blink/renderer/core/editing/frame_selection.h b/third_party/blink/renderer/core/editing/frame_selection.h
index 91da03de..48efc24 100644
--- a/third_party/blink/renderer/core/editing/frame_selection.h
+++ b/third_party/blink/renderer/core/editing/frame_selection.h
@@ -202,6 +202,10 @@
   // 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 @@
 
   GranularityStrategy* GetGranularityStrategy();
 
-  IntRect ComputeRectToScroll(RevealExtentOption);
-
   void MoveRangeSelectionInternal(const SelectionInDOMTree&, TextGranularity);
 
   // Implementation of |SynchronousMutationObserver| member functions.
diff --git a/third_party/blink/renderer/core/exported/web_frame_test.cc b/third_party/blink/renderer/core/exported/web_frame_test.cc
index a2b59917..92f6ac9 100644
--- 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 @@
   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));
diff --git a/third_party/blink/renderer/core/exported/web_view_impl.cc b/third_party/blink/renderer/core/exported/web_view_impl.cc
index ec827ff..f9e75267 100644
--- a/third_party/blink/renderer/core/exported/web_view_impl.cc
+++ b/third_party/blink/renderer/core/exported/web_view_impl.cc
@@ -2159,7 +2159,7 @@
               element->GetDocument()
                   .GetFrame()
                   ->Selection()
-                  .AbsoluteCaretBounds())),
+                  .ComputeRectToScroll(kDoNotRevealExtent))),
       ShouldZoomToLegibleScale(*element));
 
   return true;