Revert "[SpatNav] Navigation of stacked focusables"
This reverts commit 8964034e. Reason for revert: see crbug.com/961620 Original change's description: > [SpatNav] Navigation of stacked focusables > > Background: > SpatNav compares activeElement's bounding rect > with all other focusables' bounding rects. > > Example (snav-fragmented-link.html): > A multi-line ("fragmented") link has focus. > > xxxxxxLINK L > INKLINKxx #A > > Here LINK's bounding box's area is bigger than the > the combined area of its two fragments. The box, > returned by NodeRectInRootFrame(), even overlaps > another focusable, #A. We can say that LINK's > bounding box "contains" #A. > > Problem: > Spatial navigation searches _from_ the focused > element's bounding box (but not inside of it). > Here LINK's rect completely contains A's rect so > #A will never be considered as a focus candidate. > > Solution: > Give priority to focusables inside the current focus rect. > > This also solves the generic case, crbug.com/798102, > where one focusable lays "under" another one, i.e > where a focus rect "contains" (or is positioned > behind) another focusable. I test this case in > snav-focus-rect-contains-links.html. > > Previously, such containers where unreachable because > of 86d40f3d's (anno 2010): > > "... if there are 2 overlapping focusable nodes, we do > a hit test and only the node on top can get focus." > > Now, when two focusables overlap, SpatNav first goes > go the outer box. From there, focus can go to the > "on-top", or "inner" (thinking in 2D) focusables. > > Related CLs: > Search scrollers from their edges: crrev.com/589502. > This CL uses the same approach as crrev.com/589502; > when a "container" is focused, we first search inside of > it, from one of its edges. > > Distance calculation fixup: > OppositeEdge()'s returned slice will now be "just outside" > the box it is slicing. This change helps us avoid if-logic > to cover cases when focusables sit at the very top of their > container. > > Bonus cleanup: > AreElementsOnSameLine() can now be removed. It was > introduced back in 2011 at 3568f132 to support > [some] line-broken links but it didn't fix 956900. > > Bug: 798102, 956900 > Change-Id: I00aaba88a7846b36275f9913034145679576e410 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1587559 > Commit-Queue: Hugo Holgersson <hholgersson@fb.com> > Reviewed-by: David Bokan <bokan@chromium.org> > Cr-Commit-Position: refs/heads/master@{#658484} TBR=bokan@chromium.org,fs@opera.com,junho0924.seo@lge.com,hholgersson@fb.com Change-Id: If908a9fe49b9a653018ae7f9fdbf9120b130cc6d No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 798102, 956900, 961620 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1605410Reviewed-by:Dominic Battré <battre@chromium.org> Commit-Queue: Dominic Battré <battre@chromium.org> Cr-Commit-Position: refs/heads/master@{#658503}
Showing
Please register or sign in to comment