Snav: Respect a pinch-zoomed viewport

Previously we used the *layout* viewport's rectangle to determine
if a certain node was visible or not. Let's use the *visual*
viewport instead. Now:

 A visible node is a node that intersects the visual viewport.

This logic is used by spatnav to rule out offscreen focus candidates
and an offscreen activeElement. When activeElement is offscreen,
spatnav doesn't use it as its search origin; the search will start
at an edge of the visual viewport instead.

For consistency, I replaced all other uses of LayoutViewport()
with GetScrollableArea() - even though fixing HasOffscreenRect()
was enough to fix Issue 823227.

Bug: 823227
Change-Id: I8a89cfc98d896998b899a5246f2c38e23fe0171b
Reviewed-on: https://chromium-review.googlesource.com/1204094
Commit-Queue: Hugo Holgersson <hugoh@vewd.com>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588792}

third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-start-at-document-bottom.html

+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script src="resources/snav-testharness.js"></script>
+<button id="a">A</button><br>
+<button id="b" style="margin-top: 120%;">B</button>
+
+<p>When no element is focused, start searching from the document's bottom.</p>
+
+<script>
+  snav.assertSnavEnabledAndTestable();
+
+  test(() => {
+    window.scrollTo(0, document.body.scrollHeight);
+    eventSender.keyDown('ArrowUp');
+    assert_equals(document.activeElement, b);
+  }, "Spatnav picks the document's bottommost element.");
+</script>

third_party/blink/renderer/core/page/spatial_navigation.cc

@@ -77,8 +77,8 @@ FocusCandidate::FocusCandidate(Node* node, WebFocusType direction)
   }
 
   focusable_node = node;
-  is_offscreen = HasOffscreenRect(visible_node);
-  is_offscreen_after_scrolling = HasOffscreenRect(visible_node, direction);
+  is_offscreen = IsRectOffscreen(visible_node);
+  is_offscreen_after_scrolling = IsRectOffscreen(visible_node, direction);
 }
 
 bool IsSpatialNavigationEnabled(const LocalFrame* frame) {
@@ -141,47 +141,44 @@ static bool IsRectInDirection(WebFocusType direction,
   }
 }
 
-// Checks if |node| is offscreen the visible area (viewport) of its container
-// document. In case it is, one can scroll in direction or take any different
-// desired action later on.
-bool HasOffscreenRect(const Node* node, WebFocusType direction) {
-  // Get the LocalFrameView in which |node| is (which means the current viewport
-  // if |node| is not in an inner document), so we can check if its content rect
-  // is visible before we actually move the focus to it.
+// Answers true if |node| is completely outside its frames's (visual) viewport.
+// A visible node is a node that intersects the visual viewport. |direction|,
+// if given, extends the visual viewport's rect (before doing the
+// intersection-check) to also include content revealed by one scroll step in
+// that |direction|.
+
+// This logic is used by spatnav to rule out offscreen focus candidates and an
+// offscreen activeElement. When activeElement is offscreen, spatnav doesn't use
+// it as the search origin; the search will start at an edge of the visual
+// viewport instead.
+bool IsRectOffscreen(const Node* node, WebFocusType direction) {
   LocalFrameView* frame_view = node->GetDocument().View();
   if (!frame_view)
     return true;
 
   DCHECK(!frame_view->NeedsLayout());
 
-  LayoutRect container_viewport_rect(
-      frame_view->LayoutViewport()->VisibleContentRect());
-  // We want to select a node if it is currently off screen, but will be
-  // exposed after we scroll. Adjust the viewport to post-scrolling position.
-  // If the container has overflow:hidden, we cannot scroll, so we do not pass
-  // direction and we do not adjust for scrolling.
+  LayoutRect visual_viewport(
+      frame_view->GetScrollableArea()->VisibleContentRect());
+
   int pixels_per_line_step =
       ScrollableArea::PixelsPerLineStep(frame_view->GetChromeClient());
   switch (direction) {
     case kWebFocusTypeLeft:
-      container_viewport_rect.SetX(container_viewport_rect.X() -
-                                   pixels_per_line_step);
-      container_viewport_rect.SetWidth(container_viewport_rect.Width() +
-                                       pixels_per_line_step);
+      visual_viewport.SetX(visual_viewport.X() - pixels_per_line_step);
+      visual_viewport.SetWidth(visual_viewport.Width() + pixels_per_line_step);
       break;
     case kWebFocusTypeRight:
-      container_viewport_rect.SetWidth(container_viewport_rect.Width() +
-                                       pixels_per_line_step);
+      visual_viewport.SetWidth(visual_viewport.Width() + pixels_per_line_step);
       break;
     case kWebFocusTypeUp:
-      container_viewport_rect.SetY(container_viewport_rect.Y() -
-                                   pixels_per_line_step);
-      container_viewport_rect.SetHeight(container_viewport_rect.Height() +
-                                        pixels_per_line_step);
+      visual_viewport.SetY(visual_viewport.Y() - pixels_per_line_step);
+      visual_viewport.SetHeight(visual_viewport.Height() +
+                                pixels_per_line_step);
       break;
     case kWebFocusTypeDown:
-      container_viewport_rect.SetHeight(container_viewport_rect.Height() +
-                                        pixels_per_line_step);
+      visual_viewport.SetHeight(visual_viewport.Height() +
+                                pixels_per_line_step);
       break;
     default:
       break;
@@ -195,7 +192,8 @@ bool HasOffscreenRect(const Node* node, WebFocusType direction) {
   if (rect.IsEmpty())
     return true;
 
-  return !container_viewport_rect.Intersects(rect);
+  // A visible node is a node that intersects the visual viewport.
+  return !visual_viewport.Intersects(rect);
 }
 
 bool ScrollInDirection(LocalFrame* frame, WebFocusType direction) {
@@ -224,8 +222,8 @@ bool ScrollInDirection(LocalFrame* frame, WebFocusType direction) {
         return false;
     }
 
-    frame->View()->LayoutViewport()->ScrollBy(ScrollOffset(dx, dy),
-                                              kUserScroll);
+    frame->View()->GetScrollableArea()->ScrollBy(ScrollOffset(dx, dy),
+                                                 kUserScroll);
     return true;
   }
   return false;
@@ -391,7 +389,7 @@ bool CanScrollInDirection(const LocalFrame* frame, WebFocusType direction) {
   if ((direction == kWebFocusTypeUp || direction == kWebFocusTypeDown) &&
       kScrollbarAlwaysOff == vertical_mode)
     return false;
-  ScrollableArea* scrollable_area = frame->View()->LayoutViewport();
+  ScrollableArea* scrollable_area = frame->View()->GetScrollableArea();
   LayoutSize size(scrollable_area->ContentsSize());
   LayoutSize offset(scrollable_area->ScrollOffsetInt());
   LayoutRect rect(scrollable_area->VisibleContentRect(kIncludeScrollbars));
@@ -710,15 +708,16 @@ LayoutRect FindSearchStartPoint(const LocalFrame* frame,
                                 WebFocusType direction) {
   LayoutRect starting_rect = VirtualRectForDirection(
       direction,
-      frame->View()->ConvertToRootFrame(frame->View()->DocumentToFrame(
-          LayoutRect(frame->View()->LayoutViewport()->VisibleContentRect()))));
+      frame->View()->ConvertToRootFrame(
+          frame->View()->DocumentToFrame(LayoutRect(
+              frame->View()->GetScrollableArea()->VisibleContentRect()))));
 
   const Element* focused_element = frame->GetDocument()->FocusedElement();
   if (focused_element) {
     auto* area_element = ToHTMLAreaElementOrNull(focused_element);
     if (area_element)
       focused_element = area_element->ImageElement();
-    if (!HasOffscreenRect(focused_element)) {
+    if (!IsRectOffscreen(focused_element)) {
       starting_rect = area_element ? VirtualRectForAreaElementAndDirection(
                                          *area_element, direction)
                                    : NodeRectInRootFrame(focused_element, true);

third_party/blink/renderer/core/page/spatial_navigation.h

@@ -128,7 +128,7 @@ struct FocusCandidate {
   bool is_offscreen_after_scrolling;
 };
 
-bool HasOffscreenRect(const Node*, WebFocusType = kWebFocusTypeNone);
+CORE_EXPORT bool IsRectOffscreen(const Node*, WebFocusType = kWebFocusTypeNone);
 bool ScrollInDirection(LocalFrame*, WebFocusType);
 bool ScrollInDirection(Node* container, WebFocusType);
 bool IsNavigableContainer(const Node*, WebFocusType);

third_party/blink/renderer/core/page/spatial_navigation_test.cc

@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/core/page/spatial_navigation.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/core/frame/visual_viewport.h"
 #include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
 
 namespace blink {
@@ -81,4 +82,22 @@ TEST_F(SpatialNavigationTest,
   EXPECT_TRUE(IsScrollableAreaOrDocument(enclosing_container));
 }
 
+TEST_F(SpatialNavigationTest, ZooomPutsElementOffScreen) {
+  SetBodyInnerHTML(
+      "<!DOCTYPE html>"
+      "<button id='a'>hello</button><br>"
+      "<button id='b' style='margin-top: 70%'>bello</button>");
+
+  Element* a = GetDocument().getElementById("a");
+  Element* b = GetDocument().getElementById("b");
+  EXPECT_FALSE(IsRectOffscreen(a));
+  EXPECT_FALSE(IsRectOffscreen(b));
+
+  // Now, test IsRectOffscreen with a pinched viewport.
+  VisualViewport& visual_viewport = GetFrame().GetPage()->GetVisualViewport();
+  visual_viewport.SetScale(2);
+  // #b is no longer visible.
+  EXPECT_FALSE(IsRectOffscreen(a));
+  EXPECT_TRUE(IsRectOffscreen(b));
+}
 }  // namespace blink