Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(212)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 105973003: Improve spatial navigation on overlapping elements. (Closed)

Created:
7 years ago by Krzysztof Olczyk
Modified:
6 years, 8 months ago
Reviewers:
tkent, esprehn
CC:
blink-reviews
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Current heuristic spatial navigation algorithm has significant issues with overlapping elements. The algorithm ignores the elements that have non-empty intersection with current element, even if the intersection is in its transparent areas which may give an effect of elements of menu being skipped at some fancier sites. The algorithm for selecting neighbouring element was fixed as well as the distance calculation to match exactly the formula from http://www.w3.org/TR/WICD/#focus-handling which also proved to improve the user experience. BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170945

Patch Set 1 : #

Total comments: 12

Patch Set 2 : Updates layout tests, fixed raised issues. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+206 lines, -63 lines) Patch
A LayoutTests/fast/spatial-navigation/snav-overlapping-elements.html View 1 1 chunk +87 lines, -0 lines 0 comments Download
A LayoutTests/fast/spatial-navigation/snav-overlapping-elements-expected.txt View 1 1 chunk +21 lines, -0 lines 0 comments Download
M LayoutTests/fast/spatial-navigation/snav-z-index.html View 1 2 chunks +12 lines, -2 lines 0 comments Download
M LayoutTests/fast/spatial-navigation/snav-z-index-expected.txt View 1 1 chunk +11 lines, -1 line 0 comments Download
M Source/core/page/FocusController.cpp View 1 1 chunk +2 lines, -1 line 0 comments Download
M Source/core/page/SpatialNavigation.cpp View 1 9 chunks +73 lines, -59 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Krzysztof Olczyk
Hi tkent and esprehn, could you please take a look at this CL which improves ...
7 years ago (2013-12-05 13:19:35 UTC) #1
tkent
https://codereview.chromium.org/105973003/diff/20001/LayoutTests/fast/spatial-navigation/snav-overlapping-elements.html File LayoutTests/fast/spatial-navigation/snav-overlapping-elements.html (right): https://codereview.chromium.org/105973003/diff/20001/LayoutTests/fast/spatial-navigation/snav-overlapping-elements.html#newcode7 LayoutTests/fast/spatial-navigation/snav-overlapping-elements.html:7: body a { nit: You don't need to indent ...
7 years ago (2013-12-06 01:16:11 UTC) #2
tkent
The following tests failed with this CL. fast/spatial-navigation/snav-overlapping-elements.html fast/spatial-navigation/snav-z-index.html
7 years ago (2013-12-06 03:52:04 UTC) #3
Krzysztof Olczyk
Hi tkent, could you take a look at my fixes to this CL. https://codereview.chromium.org/105973003/diff/20001/LayoutTests/fast/spatial-navigation/snav-overlapping-elements.html File ...
6 years, 8 months ago (2014-04-04 11:52:14 UTC) #4
tkent
rslgtm
6 years, 8 months ago (2014-04-07 04:34:16 UTC) #5
tkent
The CQ bit was checked by tkent@chromium.org
6 years, 8 months ago (2014-04-07 04:34:19 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kolczyk@opera.com/105973003/30001
6 years, 8 months ago (2014-04-07 04:34:25 UTC) #7
commit-bot: I haz the power
6 years, 8 months ago (2014-04-07 05:43:03 UTC) #8
Message was sent while issue was closed. 
Change committed as 170945

Powered by Google App Engine
This is Rietveld 408576698