Skip to content

GitLab

T
tangled
Commit d6a58cc1 authored by Hugo Holgersson's avatar Hugo Holgersson Committed by Commit Bot
Browse files
Options

Snav: Define search origin as the first visible focus-container 

 Scenario for bug:
  1. The focused element F is inside a scrollable area A.
  2. F is offscreen (=clipped) but A is (partly) visible.

 Problem:
  Spatnav started to search from the document's edge, not
  from A's edge.

 Solution:
  Set the "search origin" to one of A's outer edges.

To solve this problem we redefine the "search origin" of
spatial navigation. The search origin is either activeElement
itself, if it's being at least partially visible, or its
first [partially] visible scroller.

If both F and its enclosing scroller A are completely
offscreen, we recurse to the scroller’s scroller up until
the root frame's document. The root document is a good base
case because it's, per definition, a visible scrollable area.

This builds onto the idea of:
https://chromium-review.googlesource.com/c/chromium/src/+/873645

Bug: 804669

Change-Id: I15b0f41426d7632fe9ec62d77a414d592e3631c0
Reviewed-on: https://chromium-review.googlesource.com/883533
Commit-Queue: Hugo Holgersson <hugoh@vewd.com>
Reviewed-by: default avatarFredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#589502}
parent 303d77a1
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 479 additions and 64 deletions
third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-search-beneath-exited-scroller.html 0 → 100644
View file @ d6a58cc1
<!DOCTYPE html>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="resources/snav-testharness.js"></script>
<style>
div {
height: 100px;
width: 100px;
overflow: scroll;
}
div p {
margin-top: 300px; /* Outside div's scrollport. */
}
</style>
<button id="a">a</button><br>
<div>
<button id='b'>b</button>
<p>some text</p>
</div>
<button id="c">c</button>
<p>Scrolling downwards puts #b off screen. When #b is
off screen, #c is reachable once the div is fully scrolled.</p>
<script>
let resultMap = [
['Down', 'a'],
['Down', 'b'],
['Down', 'b'],
['Down', 'b'],
['Down', 'b'],
['Down', 'b'],
['Down', 'b'],
['Down', 'b'],
['Down', 'b'],
['Down', 'c'],
];
snav.assertFocusMoves(resultMap);
</script>
third_party/blink/renderer/core/page/focus_controller.cc
View file @ d6a58cc1
...@@ -1444,14 +1444,15 @@ bool FocusController::AdvanceFocusDirectionally(WebFocusType direction) { ...@@ -1444,14 +1444,15 @@ bool FocusController::AdvanceFocusDirectionally(WebFocusType direction) {
if (focused_element) if (focused_element)
container = ScrollableAreaOrDocumentOf(focused_element); container = ScrollableAreaOrDocumentOf(focused_element);
const LayoutRect starting_rect = const LayoutRect visible_rect = RootViewport(current_frame);
FindSearchStartPoint(current_frame, direction); const LayoutRect start_box =
Node* pruned_sub_tree_root = nullptr; SearchOrigin(visible_rect, focused_element, direction);
bool consumed = false;
while (!consumed && container) { bool consumed = false;
Node* pruned_sub_tree_root = nullptr;
while (container) {
consumed = AdvanceFocusDirectionallyInContainer( consumed = AdvanceFocusDirectionallyInContainer(
container, starting_rect, direction, pruned_sub_tree_root); container, start_box, direction, pruned_sub_tree_root);
if (consumed) if (consumed)
break; break;
......
third_party/blink/renderer/core/page/spatial_navigation.cc
View file @ d6a58cc1
...@@ -66,8 +66,7 @@ FocusCandidate::FocusCandidate(Node* node, WebFocusType direction) ...@@ -66,8 +66,7 @@ FocusCandidate::FocusCandidate(Node* node, WebFocusType direction)
return; return;
visible_node = image; visible_node = image;
rect_in_root_frame = rect_in_root_frame = StartEdgeForAreaElement(*area, direction);
VirtualRectForAreaElementAndDirection(*area, direction);
} else { } else {
if (!node->GetLayoutObject()) if (!node->GetLayoutObject())
return; return;
...@@ -109,7 +108,7 @@ static bool RectsIntersectOnOrthogonalAxis(WebFocusType direction, ...@@ -109,7 +108,7 @@ static bool RectsIntersectOnOrthogonalAxis(WebFocusType direction,
// Return true if rect |a| is below |b|. False otherwise. // Return true if rect |a| is below |b|. False otherwise.
// For overlapping rects, |a| is considered to be below |b| // For overlapping rects, |a| is considered to be below |b|
// if both edges of |a| are below the respective ones of |b| // if both edges of |a| are below the respective ones of |b|.
static inline bool Below(const LayoutRect& a, const LayoutRect& b) { static inline bool Below(const LayoutRect& a, const LayoutRect& b) {
return a.Y() >= b.MaxY() || (a.Y() >= b.Y() && a.MaxY() > b.MaxY() && return a.Y() >= b.MaxY() || (a.Y() >= b.Y() && a.MaxY() > b.MaxY() &&
a.X() < b.MaxX() && a.MaxX() > b.X()); a.X() < b.MaxX() && a.MaxX() > b.X());
...@@ -117,7 +116,7 @@ static inline bool Below(const LayoutRect& a, const LayoutRect& b) { ...@@ -117,7 +116,7 @@ static inline bool Below(const LayoutRect& a, const LayoutRect& b) {
// Return true if rect |a| is on the right of |b|. False otherwise. // Return true if rect |a| is on the right of |b|. False otherwise.
// For overlapping rects, |a| is considered to be on the right of |b| // For overlapping rects, |a| is considered to be on the right of |b|
// if both edges of |a| are on the right of the respective ones of |b| // if both edges of |a| are on the right of the respective ones of |b|.
static inline bool RightOf(const LayoutRect& a, const LayoutRect& b) { static inline bool RightOf(const LayoutRect& a, const LayoutRect& b) {
return a.X() >= b.MaxX() || (a.X() >= b.X() && a.MaxX() > b.MaxX() && return a.X() >= b.MaxX() || (a.X() >= b.X() && a.MaxX() > b.MaxX() &&
a.Y() < b.MaxY() && a.MaxY() > b.Y()); a.Y() < b.MaxY() && a.MaxY() > b.Y());
...@@ -142,8 +141,11 @@ static bool IsRectInDirection(WebFocusType direction, ...@@ -142,8 +141,11 @@ static bool IsRectInDirection(WebFocusType direction,
} }
// Answers true if |node| is completely outside its frames's (visual) viewport. // Answers true if |node| is completely outside its frames's (visual) viewport.
// A visible node is a node that intersects the visual viewport. |direction|, // A visible node is a node that intersects the visual viewport.
// if given, extends the visual viewport's rect (before doing the // 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 // intersection-check) to also include content revealed by one scroll step in
// that |direction|. // that |direction|.
...@@ -435,7 +437,7 @@ LayoutRect NodeRectInRootFrame(const Node* node, bool ignore_border) { ...@@ -435,7 +437,7 @@ LayoutRect NodeRectInRootFrame(const Node* node, bool ignore_border) {
// This method calculates the exitPoint from the startingRect and the entryPoint // This method calculates the exitPoint from the startingRect and the entryPoint
// into the candidate rect. The line between those 2 points is the closest // into the candidate rect. The line between those 2 points is the closest
// distance between the 2 rects. Takes care of overlapping rects, defining // distance between the 2 rects. Takes care of overlapping rects, defining
// points so that the distance between them is zero where necessary // points so that the distance between them is zero where necessary.
void EntryAndExitPointsForDirection(WebFocusType direction, void EntryAndExitPointsForDirection(WebFocusType direction,
const LayoutRect& starting_rect, const LayoutRect& starting_rect,
const LayoutRect& potential_rect, const LayoutRect& potential_rect,
...@@ -654,43 +656,40 @@ bool CanBeScrolledIntoView(WebFocusType direction, ...@@ -654,43 +656,40 @@ bool CanBeScrolledIntoView(WebFocusType direction,
return true; return true;
} }
// The starting rect is the rect of the focused node, in document coordinates. // Returns a thin rectangle that represents one of box's sides.
// Compose a virtual starting rect if there is no focused node or if it is off LayoutRect OppositeEdge(WebFocusType side,
// screen. The virtual rect is the edge of the container or frame. We select const LayoutRect& box,
// which edge depending on the direction of the navigation. LayoutUnit thickness) {
LayoutRect VirtualRectForDirection(WebFocusType direction, LayoutRect thin_rect = box;
const LayoutRect& starting_rect, switch (side) {
LayoutUnit width) {
LayoutRect virtual_starting_rect = starting_rect;
switch (direction) {
case kWebFocusTypeLeft: case kWebFocusTypeLeft:
virtual_starting_rect.SetX(virtual_starting_rect.MaxX() - width); thin_rect.SetX(thin_rect.MaxX() - thickness);
virtual_starting_rect.SetWidth(width); thin_rect.SetWidth(thickness);
break;
case kWebFocusTypeUp:
virtual_starting_rect.SetY(virtual_starting_rect.MaxY() - width);
virtual_starting_rect.SetHeight(width);
break; break;
case kWebFocusTypeRight: case kWebFocusTypeRight:
virtual_starting_rect.SetWidth(width); thin_rect.SetWidth(thickness);
break; break;
case kWebFocusTypeDown: case kWebFocusTypeDown:
virtual_starting_rect.SetHeight(width); thin_rect.SetHeight(thickness);
break;
case kWebFocusTypeUp:
thin_rect.SetY(thin_rect.MaxY() - thickness);
thin_rect.SetHeight(thickness);
break; break;
default: default:
NOTREACHED(); NOTREACHED();
} }
return virtual_starting_rect; return thin_rect;
} }
LayoutRect VirtualRectForAreaElementAndDirection(const HTMLAreaElement& area, LayoutRect StartEdgeForAreaElement(const HTMLAreaElement& area,
WebFocusType direction) { WebFocusType direction) {
DCHECK(area.ImageElement()); DCHECK(area.ImageElement());
// Area elements tend to overlap more than other focusable elements. We // Area elements tend to overlap more than other focusable elements. We
// flatten the rect of the area elements to minimize the effect of overlapping // flatten the rect of the area elements to minimize the effect of overlapping
// areas. // areas.
LayoutRect rect = VirtualRectForDirection( LayoutRect rect = OppositeEdge(
direction, direction,
area.GetDocument().GetFrame()->View()->ConvertToRootFrame( area.GetDocument().GetFrame()->View()->ConvertToRootFrame(
area.ComputeAbsoluteRect(area.ImageElement()->GetLayoutObject())), area.ComputeAbsoluteRect(area.ImageElement()->GetLayoutObject())),
...@@ -704,27 +703,63 @@ HTMLFrameOwnerElement* FrameOwnerElement(FocusCandidate& candidate) { ...@@ -704,27 +703,63 @@ HTMLFrameOwnerElement* FrameOwnerElement(FocusCandidate& candidate) {
: nullptr; : nullptr;
}; };
LayoutRect FindSearchStartPoint(const LocalFrame* frame, // The rect of the visual viewport given in the root frame's coordinate space.
WebFocusType direction) { LayoutRect RootViewport(const LocalFrame* current_frame) {
LayoutRect starting_rect = VirtualRectForDirection( LocalFrameView* root_frame_view = current_frame->LocalFrameRoot().View();
direction, const LayoutRect root_doc_rect(
frame->View()->ConvertToRootFrame( root_frame_view->GetScrollableArea()->VisibleContentRect());
frame->View()->DocumentToFrame(LayoutRect( // Convert the root frame's visible rect from document space -> frame space.
frame->View()->GetScrollableArea()->VisibleContentRect())))); // For the root frame, frame space == root frame space, obviously.
LayoutRect frame_rect = root_frame_view->DocumentToFrame(root_doc_rect);
const Element* focused_element = frame->GetDocument()->FocusedElement(); return frame_rect;
if (focused_element) { }
auto* area_element = ToHTMLAreaElementOrNull(focused_element);
if (area_element) // Spatnav uses this rectangle to measure distances to focus candidates.
focused_element = area_element->ImageElement(); // The search origin is either activeElement F itself, if it's being at least
if (!IsRectOffscreen(focused_element)) { // partially visible, or else, its first [partially] visible scroller. If both
starting_rect = area_element ? VirtualRectForAreaElementAndDirection( // F and its enclosing scroller are completely off-screen, we recurse to the
*area_element, direction) // scroller’s scroller ... all the way up unil the root frame's document.
: NodeRectInRootFrame(focused_element, true); // The root frame's document is a good base case because it's, per definition,
// a visible scrollable area.
LayoutRect SearchOrigin(const LayoutRect visible_root_frame,
Node* focus_node,
const WebFocusType direction) {
if (!focus_node) {
// Search from one of the visual viewport's edges towards the navigated
// direction. For example, UP makes spatnav search upwards, starting at the
// visual viewport's bottom.
return OppositeEdge(direction, visible_root_frame);
}
LayoutRect box_in_root_frame;
LayoutUnit thickness(0);
auto* area_element = ToHTMLAreaElementOrNull(focus_node);
if (area_element) {
focus_node = area_element->ImageElement();
thickness = LayoutUnit(1); // snav-imagemap-overlapped-areas.html
box_in_root_frame =
area_element->GetDocument().GetFrame()->View()->ConvertToRootFrame(
area_element->ComputeAbsoluteRect(
area_element->ImageElement()->GetLayoutObject()));
} else {
box_in_root_frame = NodeRectInRootFrame(focus_node, true);
}
if (!IsRectOffscreen(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.
return OppositeEdge(direction,
Intersection(box_in_root_frame, visible_root_frame),
thickness);
} }
return Intersection(box_in_root_frame, visible_root_frame);
} }
return starting_rect; // Try a higher "focus-enclosing" scroller.
Node* container = ScrollableAreaOrDocumentOf(focus_node);
return SearchOrigin(visible_root_frame, container, direction);
} }
} // namespace blink } // namespace blink
third_party/blink/renderer/core/page/spatial_navigation.h
View file @ d6a58cc1
...@@ -142,14 +142,17 @@ bool AreElementsOnSameLine(const FocusCandidate& first_candidate, ...@@ -142,14 +142,17 @@ bool AreElementsOnSameLine(const FocusCandidate& first_candidate,
void DistanceDataForNode(WebFocusType, void DistanceDataForNode(WebFocusType,
const FocusCandidate& current, const FocusCandidate& current,
FocusCandidate&); FocusCandidate&);
LayoutRect NodeRectInRootFrame(const Node*, bool ignore_border = false); CORE_EXPORT LayoutRect NodeRectInRootFrame(const Node*,
LayoutRect VirtualRectForDirection(WebFocusType, bool ignore_border = false);
const LayoutRect& starting_rect, CORE_EXPORT LayoutRect OppositeEdge(WebFocusType side,
LayoutUnit width = LayoutUnit()); const LayoutRect& box,
LayoutRect VirtualRectForAreaElementAndDirection(const HTMLAreaElement&, LayoutUnit thickness = LayoutUnit());
WebFocusType); CORE_EXPORT LayoutRect RootViewport(const LocalFrame*);
LayoutRect StartEdgeForAreaElement(const HTMLAreaElement&, WebFocusType);
HTMLFrameOwnerElement* FrameOwnerElement(FocusCandidate&); HTMLFrameOwnerElement* FrameOwnerElement(FocusCandidate&);
LayoutRect FindSearchStartPoint(const LocalFrame*, WebFocusType); CORE_EXPORT LayoutRect SearchOrigin(const LayoutRect,
Node*,
const WebFocusType);
} // namespace blink } // namespace blink
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment