Commit 5c4166e2 authored by Hugo Holgersson's avatar Hugo Holgersson Committed by Commit Bot

Snav: Start at the first/last fragment when going Down/Up

When a fragmented (line broken) link has focus, we use
one of its fragments (not their union) as search origin.

Going Down: Start at the first fragment.
Going Up: Start at the last fragment.

This avoid an endless scroll loop when trying to scroll
a page downwards:

  aaaaa FFFFFF
  FFFFFFFF

When F is focused after a SCROLL, we now search below its
first fragment, instead of going back to #a. This avoids
a SCROLL-F-a-SCROLL-F-a loop.

Bug: 1015965, 1029269

Change-Id: I4703c28d59f16db4f36f2524943ee4ed067b2dd1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1912661
Commit-Queue: Hugo Holgersson <hholgersson@fb.com>
Reviewed-by: default avatarFredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#720410}
parent ce84a04d
...@@ -137,6 +137,23 @@ static bool IsRectInDirection(SpatialNavigationDirection direction, ...@@ -137,6 +137,23 @@ static bool IsRectInDirection(SpatialNavigationDirection direction,
} }
} }
bool IsFragmentedInline(Node& node) {
const LayoutObject* layout_object = node.GetLayoutObject();
if (!layout_object->IsInline() || layout_object->IsAtomicInlineLevel())
return false;
// If it has empty quads, it's most likely not a fragmented text.
// <a><div></div></a> has for example one empty rect.
Vector<FloatQuad> quads;
layout_object->AbsoluteQuads(quads);
for (const FloatQuad& quad : quads) {
if (quad.IsEmpty())
return false;
}
return quads.size() > 1;
}
FloatRect RectInViewport(const Node& node) { FloatRect RectInViewport(const Node& node) {
LocalFrameView* frame_view = node.GetDocument().View(); LocalFrameView* frame_view = node.GetDocument().View();
if (!frame_view) if (!frame_view)
...@@ -729,6 +746,50 @@ PhysicalRect RootViewport(const LocalFrame* current_frame) { ...@@ -729,6 +746,50 @@ PhysicalRect RootViewport(const LocalFrame* current_frame) {
current_frame->GetPage()->GetVisualViewport().VisibleRect()); current_frame->GetPage()->GetVisualViewport().VisibleRect());
} }
// Ignores fragments that are completely offscreen.
// Returns the first one that is not offscreen, in the given iterator range.
template <class Iterator>
PhysicalRect FirstVisibleFragment(const PhysicalRect& visibility,
Iterator fragment,
Iterator end) {
while (fragment != end) {
PhysicalRect physical_fragment(EnclosedIntRect(fragment->BoundingBox()));
physical_fragment.Intersect(visibility);
if (!physical_fragment.IsEmpty())
return physical_fragment;
++fragment;
}
return visibility;
}
PhysicalRect SearchOriginFragment(const PhysicalRect& visible_part,
const LayoutObject& fragmented,
const SpatialNavigationDirection direction) {
// For accuracy, use the first visible fragment (not the fragmented element's
// entire bounding rect which is a union of all fragments) as search origin.
Vector<FloatQuad> fragments;
fragmented.AbsoluteQuads(
fragments, kTraverseDocumentBoundaries | kApplyRemoteRootFrameOffset);
switch (direction) {
case SpatialNavigationDirection::kDown:
// Search from the topmost fragment.
return FirstVisibleFragment(visible_part, fragments.begin(),
fragments.end());
case SpatialNavigationDirection::kUp:
// Search from the bottommost fragment.
return FirstVisibleFragment(visible_part, fragments.rbegin(),
fragments.rend());
case SpatialNavigationDirection::kLeft:
// TODO(crbug.com/1029269): Return the first visible fragment.
case SpatialNavigationDirection::kRight:
// TODO(crbug.com/1029269): Return the last visible fragment.
case SpatialNavigationDirection::kNone:
break;
// Nothing to do.
}
return visible_part;
}
// Spatnav uses this rectangle to measure distances to focus candidates. // Spatnav uses this rectangle to measure distances to focus candidates.
// The search origin is either activeElement F itself, if it's being at least // The search origin is either activeElement F itself, if it's being at least
// partially visible, or else, its first [partially] visible scroller. If both // partially visible, or else, its first [partially] visible scroller. If both
...@@ -755,7 +816,14 @@ PhysicalRect SearchOrigin(const PhysicalRect& viewport_rect_of_root_frame, ...@@ -755,7 +816,14 @@ PhysicalRect SearchOrigin(const PhysicalRect& viewport_rect_of_root_frame,
return StartEdgeForAreaElement(*area_element, direction); return StartEdgeForAreaElement(*area_element, direction);
PhysicalRect box_in_root_frame = NodeRectInRootFrame(focus_node); PhysicalRect box_in_root_frame = NodeRectInRootFrame(focus_node);
return Intersection(box_in_root_frame, viewport_rect_of_root_frame); PhysicalRect visible_part =
Intersection(box_in_root_frame, viewport_rect_of_root_frame);
if (IsFragmentedInline(*focus_node)) {
return SearchOriginFragment(visible_part, *focus_node->GetLayoutObject(),
direction);
}
return visible_part;
} }
Node* container = ScrollableAreaOrDocumentOf(focus_node); Node* container = ScrollableAreaOrDocumentOf(focus_node);
...@@ -766,10 +834,8 @@ PhysicalRect SearchOrigin(const PhysicalRect& viewport_rect_of_root_frame, ...@@ -766,10 +834,8 @@ PhysicalRect SearchOrigin(const PhysicalRect& viewport_rect_of_root_frame,
return OppositeEdge(direction, Intersection(box_in_root_frame, return OppositeEdge(direction, Intersection(box_in_root_frame,
viewport_rect_of_root_frame)); viewport_rect_of_root_frame));
} }
container = ScrollableAreaOrDocumentOf(container); container = ScrollableAreaOrDocumentOf(container);
} }
return OppositeEdge(direction, viewport_rect_of_root_frame); return OppositeEdge(direction, viewport_rect_of_root_frame);
} }
......
...@@ -65,6 +65,7 @@ struct FocusCandidate { ...@@ -65,6 +65,7 @@ struct FocusCandidate {
}; };
CORE_EXPORT bool HasRemoteFrame(const Node*); CORE_EXPORT bool HasRemoteFrame(const Node*);
CORE_EXPORT bool IsFragmentedInline(Node& node);
CORE_EXPORT FloatRect RectInViewport(const Node&); CORE_EXPORT FloatRect RectInViewport(const Node&);
CORE_EXPORT bool IsOffscreen(const Node*); CORE_EXPORT bool IsOffscreen(const Node*);
CORE_EXPORT bool IsUnobscured(const FocusCandidate&); CORE_EXPORT bool IsUnobscured(const FocusCandidate&);
...@@ -86,6 +87,10 @@ CORE_EXPORT PhysicalRect RootViewport(const LocalFrame*); ...@@ -86,6 +87,10 @@ CORE_EXPORT PhysicalRect RootViewport(const LocalFrame*);
PhysicalRect StartEdgeForAreaElement(const HTMLAreaElement&, PhysicalRect StartEdgeForAreaElement(const HTMLAreaElement&,
SpatialNavigationDirection); SpatialNavigationDirection);
HTMLFrameOwnerElement* FrameOwnerElement(const FocusCandidate&); HTMLFrameOwnerElement* FrameOwnerElement(const FocusCandidate&);
CORE_EXPORT PhysicalRect
SearchOriginFragment(const PhysicalRect& visible_part,
const LayoutObject& fragmented,
const SpatialNavigationDirection direction);
CORE_EXPORT PhysicalRect SearchOrigin(const PhysicalRect&, CORE_EXPORT PhysicalRect SearchOrigin(const PhysicalRect&,
Node*, Node*,
const SpatialNavigationDirection); const SpatialNavigationDirection);
......
...@@ -620,6 +620,78 @@ TEST_F(SpatialNavigationTest, BottomOfPinchedViewport) { ...@@ -620,6 +620,78 @@ TEST_F(SpatialNavigationTest, BottomOfPinchedViewport) {
EXPECT_EQ(origin, BottomOfVisualViewport()); EXPECT_EQ(origin, BottomOfVisualViewport());
} }
TEST_F(SpatialNavigationTest, StraightTextNoFragments) {
LoadAhem();
SetBodyInnerHTML(
"<!DOCTYPE html>"
"<style>"
" body {font: 10px/10px Ahem; width: 500px}"
"</style>"
"<a href='#' id='a'>blaaaaa blaaaaa blaaaaa</a>");
Element* a = GetDocument().getElementById("a");
EXPECT_FALSE(IsFragmentedInline(*a));
}
TEST_F(SpatialNavigationTest, LineBrokenTextHasFragments) {
LoadAhem();
SetBodyInnerHTML(
"<!DOCTYPE html>"
"<style>"
" body {font: 10px/10px Ahem; width: 40px}"
"</style>"
"<a href='#' id='a'>blaaaaa blaaaaa blaaaaa</a>");
Element* a = GetDocument().getElementById("a");
EXPECT_TRUE(IsFragmentedInline(*a));
}
TEST_F(SpatialNavigationTest, ManyClientRectsButNotLineBrokenText) {
SetBodyInnerHTML(
"<!DOCTYPE html>"
"<style>"
" div {width: 20px; height: 20px;}"
"</style>"
"<a href='#' id='a'><div></div></a>");
Element* a = GetDocument().getElementById("a");
EXPECT_FALSE(IsFragmentedInline(*a));
}
TEST_F(SpatialNavigationTest, UseTheFirstFragment) {
LoadAhem();
SetBodyInnerHTML(
"<!DOCTYPE html>"
"<style>"
" body {font: 10px/10px Ahem; margin: 0; width: 50px;}"
" div {width: 20px; height: 20px;}"
"</style>"
"<a href='#' id='a'>12345 12</a>");
Element* a = GetDocument().getElementById("a");
EXPECT_TRUE(IsFragmentedInline(*a));
// Search downards.
PhysicalRect origin_down = SearchOrigin(RootViewport(&GetFrame()), a,
SpatialNavigationDirection::kDown);
PhysicalRect origin_fragment =
SearchOriginFragment(NodeRectInRootFrame(a), *a->GetLayoutObject(),
SpatialNavigationDirection::kDown);
EXPECT_EQ(origin_down, origin_fragment);
EXPECT_EQ(origin_down.Height(), 10);
EXPECT_EQ(origin_down.Width(), 50);
EXPECT_EQ(origin_down.X(), 0);
EXPECT_EQ(origin_down.Y(), 0);
// Search upwards.
PhysicalRect origin_up = SearchOrigin(RootViewport(&GetFrame()), a,
SpatialNavigationDirection::kUp);
PhysicalRect origin_fragment_up =
SearchOriginFragment(NodeRectInRootFrame(a), *a->GetLayoutObject(),
SpatialNavigationDirection::kUp);
EXPECT_EQ(origin_up, origin_fragment_up);
EXPECT_EQ(origin_up.Height(), 10);
EXPECT_EQ(origin_up.Width(), 20);
EXPECT_EQ(origin_up.X(), 0);
EXPECT_EQ(origin_up.Y(), 10);
}
TEST_F(SpatialNavigationTest, TopOfPinchedViewport) { TEST_F(SpatialNavigationTest, TopOfPinchedViewport) {
PhysicalRect origin = SearchOrigin(RootViewport(&GetFrame()), nullptr, PhysicalRect origin = SearchOrigin(RootViewport(&GetFrame()), nullptr,
SpatialNavigationDirection::kDown); SpatialNavigationDirection::kDown);
......
<!doctype html>
<style>
body {margin: 0; width: 420px;}
a {font-size: 30px; font-family: Ahem;}
a:focus {outline: 5px solid orange;}
</style>
<a href="#" id="a">aaaaaaaaaaaa aa</a><a href="#" id="b" style="color: pink">bb</a><br>
<a href="#" id="c" style="color: lightgreen;">cc</a>
<div id="spacer" tabindex="0" style="margin-bottom: 120vh">
When going downwards and the first fragment is hidden, the first *visible* fragment becomes origin, not the first fragment.
</div>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="resources/snav-testharness.js"></script>
<script>
a.focus();
window.scroll(0, 45);
// Now only #a's second fragment is visible.
var resultMap = [
["Down", "c"], // #c is closest to the visible fragment.
["Up", "a"], // Focus and scroll all of #a into view.
["Down", "b"], // Now that all of #a is visible, we search from the first fragment.
];
document.getElementById("a").focus();
snav.assertFocusMoves(resultMap);
</script>
\ No newline at end of file
<!doctype html>
<style>
body {margin: 0; width: 280px;}
a {font-size: 30px; font-family: Ahem;}
a:focus {outline: 5px solid orange;}
</style>
<a dir="rtl" id="a" href="#">عربية and עברית</a><a href="#" id="b" style="color: pink">bb</a><br>
<a href="#" id="c" style="color: lightgreen;">cc</a>
<div id="spacer" tabindex="0" style="margin-bottom: 120vh">
For rtl-text, when going downwards and the first fragment is hidden, the first *visible* fragment becomes origin, not the first fragment.
</div>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="resources/snav-testharness.js"></script>
<script>
a.focus();
window.scroll(0, 45);
// Now only #a's second fragment is visible.
var resultMap = [
["Down", "c"], // #c is closest to the visible fragment.
["Up", "a"], // Focus and scroll all of #a into view.
["Down", "b"], // Now that all of #a is visible, we search from the first fragment.
];
document.getElementById("a").focus();
snav.assertFocusMoves(resultMap);
</script>
\ No newline at end of file
<!doctype html>
<style>
p {font-size: 30px; font-family: Ahem; width: 420px; margin-bottom: 200vh; margin-top: 0px;}
a:focus {outline: 5px solid orange;}
a#A {color: teal;}
</style>
The long link spans over several lines. The first line has another link. There are no focusables below the two links.
<p><a href="#" id="a">aaa</a> xx <a href="#" id="b">bbbbb bbbbbbbbbbbbbb bbbb</a></p>
Some other content down here. This content must be reachable through spatial navigation.
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="resources/snav-testharness.js"></script>
<script>
var resultMap = [
["Down", "a"], // Scroll.
["Down", "a"], // Scroll.
["Down", "b"], // Move focus and scroll.
["Down", "b"], // Scroll.
["Down", "b"], // Scroll.
["Down", "b"], // Scroll.
];
document.getElementById("a").focus();
snav.assertFocusMoves(resultMap);
</script>
\ No newline at end of file
...@@ -61,8 +61,7 @@ This bounding box covers another link. Here we test that SpatNav can reach that ...@@ -61,8 +61,7 @@ This bounding box covers another link. Here we test that SpatNav can reach that
["Right", "short"], // We searched *inside* #fragmentedB's box. ["Right", "short"], // We searched *inside* #fragmentedB's box.
["Down", "end"], // This avoids an endless loop between #fragmentedB and #short if Down is held. ["Down", "end"], // This avoids an endless loop between #fragmentedB and #short if Down is held.
["Up", "fragmentedB"], ["Up", "fragmentedB"],
["Down", "short"], // We searched *inside* #fragmentedB's box. ["Down", "now_reachableC"], // We searched *inside* #fragmentedB's box.
["Right", "now_reachableC"],
["Right", "anotherC"], ["Right", "anotherC"],
["Left", "fragmentedB"], ["Left", "fragmentedB"],
["Left", "now_reachableC"], // We searched *inside* #fragmentedB's box. ["Left", "now_reachableC"], // We searched *inside* #fragmentedB's box.
......
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