Skip to content

GitLab

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

Snav: Start at the first/last fragment when going Left/Right 

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

Going Left: Start at the beginning of the link (the first fragment).
Going Right: Start at the end of the link (the last fragment).

This feels more intuitive and is probably a better UX for
most users since most texts are written left-to-right.

Discussed in: crrev.com/c/1912661

Bug: 1029269
Change-Id: I2ba214a3208ce82d3b6cb6a85b6840b3d9c33961
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1944169
Commit-Queue: Hugo Holgersson <hholgersson@fb.com>
Reviewed-by: default avatarFredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#720429}
parent f1f7ebbb
Hide whitespace changes
Inline Side-by-side
Showing with 16 additions and 8 deletions
third_party/blink/renderer/core/page/spatial_navigation.cc
View file @ c51b9f5e
...@@ -771,18 +771,16 @@ PhysicalRect SearchOriginFragment(const PhysicalRect& visible_part, ...@@ -771,18 +771,16 @@ PhysicalRect SearchOriginFragment(const PhysicalRect& visible_part,
fragmented.AbsoluteQuads( fragmented.AbsoluteQuads(
fragments, kTraverseDocumentBoundaries | kApplyRemoteRootFrameOffset); fragments, kTraverseDocumentBoundaries | kApplyRemoteRootFrameOffset);
switch (direction) { switch (direction) {
case SpatialNavigationDirection::kLeft:
case SpatialNavigationDirection::kDown: case SpatialNavigationDirection::kDown:
// Search from the topmost fragment. // Search from the topmost fragment.
return FirstVisibleFragment(visible_part, fragments.begin(), return FirstVisibleFragment(visible_part, fragments.begin(),
fragments.end()); fragments.end());
case SpatialNavigationDirection::kRight:
case SpatialNavigationDirection::kUp: case SpatialNavigationDirection::kUp:
// Search from the bottommost fragment. // Search from the bottommost fragment.
return FirstVisibleFragment(visible_part, fragments.rbegin(), return FirstVisibleFragment(visible_part, fragments.rbegin(),
fragments.rend()); 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: case SpatialNavigationDirection::kNone:
break; break;
// Nothing to do. // Nothing to do.
......
third_party/blink/renderer/core/page/spatial_navigation_test.cc
View file @ c51b9f5e
...@@ -661,7 +661,6 @@ TEST_F(SpatialNavigationTest, UseTheFirstFragment) { ...@@ -661,7 +661,6 @@ TEST_F(SpatialNavigationTest, UseTheFirstFragment) {
"<!DOCTYPE html>" "<!DOCTYPE html>"
"<style>" "<style>"
" body {font: 10px/10px Ahem; margin: 0; width: 50px;}" " body {font: 10px/10px Ahem; margin: 0; width: 50px;}"
" div {width: 20px; height: 20px;}"
"</style>" "</style>"
"<a href='#' id='a'>12345 12</a>"); "<a href='#' id='a'>12345 12</a>");
Element* a = GetDocument().getElementById("a"); Element* a = GetDocument().getElementById("a");
...@@ -690,6 +689,16 @@ TEST_F(SpatialNavigationTest, UseTheFirstFragment) { ...@@ -690,6 +689,16 @@ TEST_F(SpatialNavigationTest, UseTheFirstFragment) {
EXPECT_EQ(origin_up.Width(), 20); EXPECT_EQ(origin_up.Width(), 20);
EXPECT_EQ(origin_up.X(), 0); EXPECT_EQ(origin_up.X(), 0);
EXPECT_EQ(origin_up.Y(), 10); EXPECT_EQ(origin_up.Y(), 10);
// Search from the top fragment.
PhysicalRect origin_left = SearchOrigin(RootViewport(&GetFrame()), a,
SpatialNavigationDirection::kLeft);
EXPECT_EQ(origin_left, origin_down);
// Search from the bottom fragment.
PhysicalRect origin_right = SearchOrigin(RootViewport(&GetFrame()), a,
SpatialNavigationDirection::kRight);
EXPECT_EQ(origin_right, origin_up);
} }
TEST_F(SpatialNavigationTest, TopOfPinchedViewport) { TEST_F(SpatialNavigationTest, TopOfPinchedViewport) {
......
third_party/blink/web_tests/fast/spatial-navigation/snav-fragmented-link.html
View file @ c51b9f5e
...@@ -45,7 +45,8 @@ This bounding box covers another link. Here we test that SpatNav can reach that ...@@ -45,7 +45,8 @@ This bounding box covers another link. Here we test that SpatNav can reach that
["Down", "anotherC"], // #anotherA and #anotherC are perfectly aligned. ["Down", "anotherC"], // #anotherA and #anotherC are perfectly aligned.
["Up", "anotherA"], ["Up", "anotherA"],
["Left", "fragmentedA"], ["Left", "fragmentedA"],
["Left", "now_reachableB"], // We searched *inside* #fragmentedA's box. ["Right", "now_reachableA"], // We searched *inside* #fragmentedA's bounding box.
["Right", "now_reachableB"],
["Left", "now_reachableA"], ["Left", "now_reachableA"],
["Left", "B"], // We don't go to the enclosing #fragmentedA. If we did, holding ["Left", "B"], // We don't go to the enclosing #fragmentedA. If we did, holding
// Left would cause a loop inside #fragmentedA's bounding box. // Left would cause a loop inside #fragmentedA's bounding box.
...@@ -58,13 +59,13 @@ This bounding box covers another link. Here we test that SpatNav can reach that ...@@ -58,13 +59,13 @@ This bounding box covers another link. Here we test that SpatNav can reach that
// This avoids an endless loop between #fragmentedB and #short if Right is held. // This avoids an endless loop between #fragmentedB and #short if Right is held.
["Down", "anotherC"], ["Down", "anotherC"],
["Left", "fragmentedB"], ["Left", "fragmentedB"],
["Right", "short"], // We searched *inside* #fragmentedB's box. ["Right", "now_reachableC"], // We searched from the last fragment's end.
["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", "now_reachableC"], // We searched *inside* #fragmentedB's box. ["Down", "now_reachableC"], // We searched *inside* #fragmentedB's box.
["Right", "anotherC"], ["Right", "anotherC"],
["Left", "fragmentedB"], ["Left", "fragmentedB"],
["Left", "now_reachableC"], // We searched *inside* #fragmentedB's box. ["Left", "short"], // We searched *inside* #fragmentedB's bounding box, from the first fragment.
["Down", "end"] ["Down", "end"]
]; ];
snav.assertFocusMoves(resultMap); snav.assertFocusMoves(resultMap);
......
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