Snav: Test visibility through the visual viewport

Scenario:
activeElement (focus) resides in an *offscreen* iframe's viewport.

Problem:
Spatnav only checked if activeElement was visible in its localmost
frame. Even if that frame wasn't visible to the user, activeElement
was used as the starting point in the search for focus candidates.

Solution:
Do not only ensure that activeElement is visible in its own frame:
Also ensure that activeElement is being visible through the root
frame. We do this by testing visibility through VisualViewport.

Note:
Element::VisibleBoundsInVisualViewport() returns an empty rect if
an element is clipped by an iframe but VBIVV does not return an empty
rect if the element is clipped by an overflow div. That's why we
first still check the node's LayoutObject's VisualRectInDocument().
Let's fix this problem in Issue 889840.

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

third_party/blink/renderer/core/dom/element.cc

@@ -1299,9 +1299,9 @@ IntRect Element::VisibleBoundsInVisualViewport() const {
   rect.Intersect(frame_clip_rect);
 
   // MapToVisualRectInAncestorSpace, called with a null ancestor argument,
-  // returns the viewport-visible rect in the local frame root's coordinates,
-  // accounting for clips and transformed in embedding containers. This
-  // includes clips that might be applied by out-of-process frame ancestors.
+  // returns the viewport-visible rect in the root frame's coordinate space.
+  // MapToVisualRectInAncestorSpace applies ancestors' frame's clipping but does
+  // not apply (overflow) element clipping.
   GetDocument().View()->GetLayoutView()->MapToVisualRectInAncestorSpace(
       nullptr, rect, kUseTransforms | kTraverseDocumentBoundaries,
       kDefaultVisualRectFlags);

third_party/blink/renderer/core/dom/element.h

@@ -322,12 +322,10 @@ class CORE_EXPORT Element : public ContainerNode {
   virtual void scrollTo(const ScrollToOptions&);
 
   IntRect BoundsInViewport() const;
-  // For elements that are not contained in any OOPIFs, this returns an
-  // intersection rectangle of the bounds rectangle and the viewport
-  // rectangle, in the visual viewport coordinate. For elements within OOPIFs,
-  // the returned rect is the intersection with the viewport but in the
-  // coordinate space of the local frame root.
-  // This function is used to show popups beside this element.
+  // Returns an intersection rectangle of the bounds rectangle and the visual
+  // viewport's rectangle in the visual viewport's coordinate space.
+  // Applies ancestors' frames' clipping, but does not (yet) apply (overflow)
+  // element clipping (crbug.com/889840).
   IntRect VisibleBoundsInVisualViewport() const;
 
   DOMRectList* getClientRects();

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

@@ -77,8 +77,9 @@ FocusCandidate::FocusCandidate(Node* node, WebFocusType direction)
   }
 
   focusable_node = node;
-  is_offscreen = IsRectOffscreen(visible_node);
-  is_offscreen_after_scrolling = IsRectOffscreen(visible_node, direction);
+  is_offscreen = IsOffscreen(visible_node);
+  is_offscreen_after_scrolling =
+      IsOffscreenAfterFrameScroll(visible_node, direction);
 }
 
 bool IsSpatialNavigationEnabled(const LocalFrame* frame) {
@@ -141,47 +142,81 @@ static bool IsRectInDirection(WebFocusType direction,
   }
 }
 
-// Answers true if |node| is completely outside its frames's (visual) viewport.
-// A visible node is a node that intersects the visual viewport.
-// TODO(crbug.com/881721): Intersect the user's visual viewport, not the node's
-// frame's 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|.
-
+// Answers true if |node| is completely outside the user's (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 the search origin; the search will start at an edge of the visual
 // viewport instead.
-bool IsRectOffscreen(const Node* node, WebFocusType direction) {
+// TODO(crbug.com/889840): Fix VisibleBoundsInVisualViewport().
+// If VisibleBoundsInVisualViewport() would have taken "element-clips" into
+// account, spatnav could have called it directly; no need to check the
+// LayoutObject's VisibleContentRect.
+bool IsOffscreen(const Node* node) {
+  LocalFrameView* frame_view = node->GetDocument().View();
+  if (!frame_view)
+    return true;
+
+  DCHECK(!frame_view->NeedsLayout());
+
+  LayoutRect frame_viewport(
+      frame_view->GetScrollableArea()->VisibleContentRect());
+
+  LayoutObject* layout_object = node->GetLayoutObject();
+  if (!layout_object)
+    return true;
+
+  LayoutRect rect(layout_object->VisualRectInDocument());
+  if (rect.IsEmpty())
+    return true;
+
+  if (!frame_viewport.Intersects(rect))
+    return true;
+
+  // Now we know that the node is visible in the its own frame's viewport (it is
+  // not clipped by a scrollable div). That is, we've taken "element-clipping"
+  // into account - now we only need to ensure that this node isn't clipped by
+  // a frame.
+  IntRect rect_in_root_frame;
+  if (node->IsDocumentNode() && ToDocument(*node).body())
+    node = ToDocument(*node).body();
+  if (node->IsElementNode())
+    rect_in_root_frame = ToElement(*node).VisibleBoundsInVisualViewport();
+  return rect_in_root_frame.IsEmpty();
+}
+
+// As IsOffscreen() but returns visibility through the |node|'s frame's viewport
+// after scrolling the frame in |direction|.
+bool IsOffscreenAfterFrameScroll(const Node* node, WebFocusType direction) {
   LocalFrameView* frame_view = node->GetDocument().View();
   if (!frame_view)
     return true;
 
   DCHECK(!frame_view->NeedsLayout());
 
-  LayoutRect visual_viewport(
+  // If |node| is in the root frame, VisibleContentRect() will include
+  // visual viewport transformation (pinch-zoom) if one exists.
+  LayoutRect frame_viewport(
       frame_view->GetScrollableArea()->VisibleContentRect());
 
+  // |direction| extends the node's frame's viewport's rect (before doing the
+  // intersection-check) to also include content revealed by one scroll step in
+  // that |direction|.
   int pixels_per_line_step =
       ScrollableArea::PixelsPerLineStep(frame_view->GetChromeClient());
   switch (direction) {
     case kWebFocusTypeLeft:
-      visual_viewport.SetX(visual_viewport.X() - pixels_per_line_step);
-      visual_viewport.SetWidth(visual_viewport.Width() + pixels_per_line_step);
+      frame_viewport.SetX(frame_viewport.X() - pixels_per_line_step);
+      frame_viewport.SetWidth(frame_viewport.Width() + pixels_per_line_step);
       break;
     case kWebFocusTypeRight:
-      visual_viewport.SetWidth(visual_viewport.Width() + pixels_per_line_step);
+      frame_viewport.SetWidth(frame_viewport.Width() + pixels_per_line_step);
       break;
     case kWebFocusTypeUp:
-      visual_viewport.SetY(visual_viewport.Y() - pixels_per_line_step);
-      visual_viewport.SetHeight(visual_viewport.Height() +
-                                pixels_per_line_step);
+      frame_viewport.SetY(frame_viewport.Y() - pixels_per_line_step);
+      frame_viewport.SetHeight(frame_viewport.Height() + pixels_per_line_step);
       break;
     case kWebFocusTypeDown:
-      visual_viewport.SetHeight(visual_viewport.Height() +
-                                pixels_per_line_step);
+      frame_viewport.SetHeight(frame_viewport.Height() + pixels_per_line_step);
       break;
     default:
       break;
@@ -195,8 +230,7 @@ bool IsRectOffscreen(const Node* node, WebFocusType direction) {
   if (rect.IsEmpty())
     return true;
 
-  // A visible node is a node that intersects the visual viewport.
-  return !visual_viewport.Intersects(rect);
+  return !frame_viewport.Intersects(rect);
 }
 
 bool HasRemoteFrame(const Node* node) {
@@ -751,7 +785,7 @@ LayoutRect SearchOrigin(const LayoutRect viewport_rect_of_root_frame,
     box_in_root_frame = NodeRectInRootFrame(focus_node, true);
   }
 
-  if (!IsRectOffscreen(focus_node)) {
+  if (!IsOffscreen(focus_node)) {
     // We found the first box that encloses focus and is [partially] visible.
     if (area_element || IsScrollableAreaOrDocument(focus_node)) {
       // When searching a container, we start from one of its sides.

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

@@ -83,8 +83,9 @@ struct FocusCandidate {
   bool is_offscreen_after_scrolling;
 };
 
-CORE_EXPORT bool IsRectOffscreen(const Node*, WebFocusType = kWebFocusTypeNone);
 CORE_EXPORT bool HasRemoteFrame(const Node*);
+CORE_EXPORT bool IsOffscreen(const Node*);
+CORE_EXPORT bool IsOffscreenAfterFrameScroll(const Node*, WebFocusType);
 bool ScrollInDirection(LocalFrame*, WebFocusType);
 bool ScrollInDirection(Node* container, WebFocusType);
 bool IsNavigableContainer(const Node*, WebFocusType);