Skip to content

GitLab

T
tangled
Commit 81ae0377 authored by Jeonghee Ahn's avatar Jeonghee Ahn Committed by Commit Bot
Browse files
Options

[SpatNav] Prevent duplicate key handling. 

Some web-sites may handle the arrow keys themselves for moving focus.
Unfortunately some of them do not call preventDefault.

In this case, the focus moves twice.
(ex. Buttons of https://expedia.com (class="tabs cf col"))

This patch prevents duplicate focus moving by checking that
the current fucused element matches the event target element.

Change-Id: If97fd4d81f3eabf7fc015aa464b6d121f30458fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1859425
Commit-Queue: Jeonghee Ahn <jeonghee27.ahn@lge.com>
Reviewed-by: default avatarFredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#706853}
parent 68254386
Hide whitespace changes
Inline Side-by-side
Showing with 158 additions and 1 deletion
third_party/blink/renderer/core/page/spatial_navigation_controller.cc
View file @ 81ae0377
......@@ -138,12 +138,20 @@ bool SpatialNavigationController::HandleArrowKeyboardEvent(
if (direction == SpatialNavigationDirection::kNone)
return false;
// If the focus has already moved by a previous handler, return false.
const Element* focused = GetFocusedElement();
if (focused && focused != event->target()) {
// SpatNav does not need to handle this arrow key because
// the webpage had a key-handler that already moved focus.
return false;
}
// In focusless mode, the user must explicitly move focus in and out of an
// editable so we can avoid advancing interest and we should swallow the
// event. This prevents double-handling actions for things like search box
// suggestions.
if (RuntimeEnabledFeatures::FocuslessSpatialNavigationEnabled()) {
if (Element* focused = GetFocusedElement()) {
if (focused) {
if (HasEditableStyle(*focused) || focused->IsTextControl())
return true;
}
......
third_party/blink/web_tests/fast/spatial-navigation/snav-check-target-element.html 0 → 100644
View file @ 81ae0377
<!DOCTYPE html>
<!-- TODO(crbug.com/1015307): should adopt title value. -->
<title>SpatNav isn't performed if the webpage directly handles the arrow keys for moving focus.</title>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="resources/snav-testharness.js"></script>
<button id="a">A</button>
<button id="b">B</button>
<button id="c">C</button>
<button id="d">D</button>
<button id="e">E</button>
<button id="f">F</button>
<button id="g">G</button>
<script>
const buttons = document.getElementsByTagName("button");
// Keydown on D button.
buttons[3].addEventListener('keydown', (e) => {
// Custom SpatNav on web page.
if (e.key == 'ArrowLeft') {
// Move focus to B button.
buttons[1].focus();
} else if (e.key == 'ArrowRight') {
// Move focus to F button.
buttons[5].focus();
}
});
</script>
<script>
var resultMap = [
["Right", "a"],
["Right", "b"],
["Right", "c"],
["Right", "d"],
["Right", "f"],
["Right", "g"],
["Left", "f"],
["Left", "e"],
["Left", "d"],
["Left", "b"],
["Left", "a"]
];
snav.assertFocusMoves(resultMap);
</script>
third_party/blink/web_tests/fast/spatial-navigation/snav-prevent-default.html 0 → 100644
View file @ 81ae0377
<!DOCTYPE html>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="resources/snav-testharness.js"></script>
<div>
<button id="a">A</button>
</div>
<div>
<button id="b">B</button>
</div>
<div>
<button id="c">C</button>
</div>
<div tabindex=-1>
<button id="d">D</button>
</div>
<div tabindex=-1>
<button id="e">E</button>
</div>
<div tabindex=-1>
<button id="f">F</button>
</div>
<div>
<button id="g">G</button>
</div>
<script>
const buttons = document.getElementsByTagName("button");
const parentContainers = document.getElementsByTagName("div");
const preventDefault = (e) => e.preventDefault();
// Add event handler.
buttons[0].addEventListener('keydown', preventDefault);
buttons[1].addEventListener('keydown', preventDefault, { capture: true });
buttons[2].addEventListener('keydown', preventDefault, { passive: true });
parentContainers[3].addEventListener('keydown', preventDefault);
parentContainers[4].addEventListener('keydown', preventDefault, { capture: true });
parentContainers[5].addEventListener('keydown', preventDefault, { passive: true });
buttons[6].addEventListener('keydown', (e) => { });
// SpatNav Test
snav.assertSnavEnabledAndTestable();
test(() => {
// Move focus to A button.
snav.triggerMove('Down');
assert_equals(window.internals.interestedElement,
buttons[0], "A button should start off interested.");
snav.triggerMove('Down');
assert_equals(window.internals.interestedElement,
buttons[0], "The focus should remain on the A button.");
// Move focus to B button.
buttons[1].focus();
assert_equals(window.internals.interestedElement,
buttons[1], "A button should start off interested.");
snav.triggerMove('Down');
assert_equals(window.internals.interestedElement,
buttons[1], "The focus should remain on the B button.");
// Move focus to C button.
buttons[2].focus();
assert_equals(window.internals.interestedElement,
buttons[2], "C button should start off interested.");
// Move focus to D button.
snav.triggerMove('Down');
assert_equals(window.internals.interestedElement,
buttons[3], "The focus should move to the D button.");
snav.triggerMove('Down');
assert_equals(window.internals.interestedElement,
buttons[3], "The focus should remain on the D button.");
// Move focus to E button.
buttons[4].focus();
assert_equals(window.internals.interestedElement,
buttons[4], "E button should start off interested.");
snav.triggerMove('Down');
assert_equals(window.internals.interestedElement,
buttons[4], "The focus should remain on the E button.");
// Move focus to F button.
buttons[5].focus();
assert_equals(window.internals.interestedElement,
buttons[5], "E button should start off interested.");
// Move focus to G button.
snav.triggerMove('Down');
assert_equals(window.internals.interestedElement,
buttons[6], "The focus should move to the G button.");
// Move focus to F button again.
snav.triggerMove('Up');
assert_equals(window.internals.interestedElement,
buttons[5], "The focus should move to the F button.");
}, "Event.preventDefault() prevent the spatial-navigation operation.");
</script>
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