Skip to content

GitLab

T
tangled
Commit d88bd9df authored by Alice Boxhall's avatar Alice Boxhall Committed by Commit Bot
Browse files
Options

Make focus-visible match on keypress 

Change-Id: I7f467623d5476f717d73aeeaecdc69399c6b65f5
Reviewed-on: https://chromium-review.googlesource.com/1074584
Commit-Queue: Alice Boxhall <aboxhall@chromium.org>
Reviewed-by: default avatarJonathon Kereliuk <kereliuk@chromium.org>
Reviewed-by: default avatarRune Lillesveen <futhark@chromium.org>
Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564977}
parent 736a44b2
Hide whitespace changes
Inline Side-by-side
Showing with 117 additions and 18 deletions
third_party/WebKit/LayoutTests/TestExpectations
View file @ d88bd9df
...@@ -4791,3 +4791,6 @@ crbug.com/849284 [ Mac ] http/tests/devtools/editor/text-editor-ctrl-d-2.js [ Ti ...@@ -4791,3 +4791,6 @@ crbug.com/849284 [ Mac ] http/tests/devtools/editor/text-editor-ctrl-d-2.js [ Ti
# Sheriff 2018-06-06 # Sheriff 2018-06-06
crbug.com/849975 external/wpt/css/css-animations/Element-getAnimations.tentative.html [ Pass Failure ] crbug.com/849975 external/wpt/css/css-animations/Element-getAnimations.tentative.html [ Pass Failure ]
# This won't pass as a layout test until the linked bug is fixed.
crbug.com/828858 external/wpt/css/selectors/focus-visible-007.html [ Failure ]
third_party/WebKit/LayoutTests/external/wpt/css/selectors/focus-visible-007.html 0 → 100644
View file @ d88bd9df
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8" />
<title>CSS Test (Selectors): Keyboard focus enables :focus-visible</title>
<link rel="author" title="Alice Boxhall" href="aboxhall@chromium.org" />
<link rel="help" href="https://drafts.csswg.org/selectors-4/#the-focus-visible-pseudo" />
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<style>
body[data-hadkeydown] #one:focus-visible { background-color: rgb(128, 196, 128); }
body:not([data-hadkeydown]) #one:focus-visible { background-color: rgb(196, 128, 128); }
body:not([data-hadkeydown]) #two:focus-visible { background-color: rgb(196, 128, 128); }
body:not([data-hadkeydown]) #two:focus:not(:focus-visible) {
background-color: rgb(128, 196, 128);
}
</style>
</head>
<body>
<div id="one" tabindex="0">Click me first.</div>
<div id="two" tabindex="0">Click me second.</div>
<script>
document.body.addEventListener("keydown", (e) => {
delete document.body.dataset.hadmousedown;
document.body.dataset.hadkeydown = "";
}, true);
document.body.addEventListener("mousedown", (e) => {
document.body.dataset.hadmousedown = "";
}, true);
document.body.addEventListener("focus", (e) => {
if ("hadmousedown" in document.body.dataset) {
delete document.body.dataset.hadmousedown;
delete document.body.dataset.hadkeydown;
}
}, true);
async_test(async function(t) {
let tested_modality_change = false;
let tested_modality_unchanged_by_mouse_click = false;
let tested_mouse_focus_after_modality_change = false;
// TODO(crbug.com/828858): Remove this check once bug is resolved.
test_driver.send_keys(document.body, "\uE050").then(t.step_func(() => {
const handle_initial_focus = t.step_func(() => {
one.addEventListener("keyup", t.step_func(test_modality_change));
one.removeEventListener("focus", handle_initial_focus);
test_driver.send_keys(one, "\uE050");
});
const test_modality_change = t.step_func(() => {
assert_equals(getComputedStyle(one).backgroundColor, "rgb(128, 196, 128)");
tested_modality_change = true;
one.removeEventListener("keyup", test_modality_change);
one.addEventListener("click", test_modality_unchanged_by_mouse_click);
test_driver.click(one);
});
const test_modality_unchanged_by_mouse_click = t.step_func(() => {
assert_true(tested_modality_change);
assert_equals(getComputedStyle(one).backgroundColor, "rgb(128, 196, 128)");
tested_modality_unchanged_by_mouse_click = true;
one.removeEventListener("click", test_modality_unchanged_by_mouse_click);
two.addEventListener("focus", test_mouse_focus_after_modality_change);
test_driver.click(two);
});
const test_mouse_focus_after_modality_change = t.step_func(() => {
assert_true(tested_modality_unchanged_by_mouse_click);
assert_false("hadkeydown" in document.body.dataset);
assert_equals(getComputedStyle(two).backgroundColor, "rgb(128, 196, 128)");
tested_mouse_focus_after_modality_change = true;
t.done();
});
one.addEventListener("focus", handle_initial_focus);
test_driver.click(one);
})).catch(t.step_func((e) => {
// TODO(crbug.com/828858): Remove this check once bug is resolved.
assert_true(false, "send_keys not implmented yet");
t.done();
}));
}, "Using keyboard while element is focused should trigger :focus-visible; using mouse without moving focus should not cancel it; moving focus using mouse should cancel it.");
</script>
</body>
</html>
third_party/blink/renderer/core/css/selector_checker.cc
View file @ d88bd9df
...@@ -1379,14 +1379,16 @@ bool SelectorChecker::MatchesFocusVisiblePseudoClass(const Element& element) { ...@@ -1379,14 +1379,16 @@ bool SelectorChecker::MatchesFocusVisiblePseudoClass(const Element& element) {
if (force_pseudo_state) if (force_pseudo_state)
return true; return true;
const Document& document = element.GetDocument();
bool always_show_focus_ring = element.MayTriggerVirtualKeyboard(); bool always_show_focus_ring = element.MayTriggerVirtualKeyboard();
bool last_focus_from_mouse = bool last_focus_from_mouse =
element.GetDocument().GetFrame() && document.GetFrame() &&
element.GetDocument().GetFrame()->Selection().FrameIsFocusedAndActive() && document.GetFrame()->Selection().FrameIsFocusedAndActive() &&
element.GetDocument().LastFocusType() == kWebFocusTypeMouse; document.LastFocusType() == kWebFocusTypeMouse;
bool had_keyboard_event = document.HadKeyboardEvent();
return element.IsFocused() && return element.IsFocused() && (!last_focus_from_mouse || had_keyboard_event ||
(!last_focus_from_mouse || always_show_focus_ring); always_show_focus_ring);
} }
} // namespace blink } // namespace blink
third_party/blink/renderer/core/dom/node.cc
View file @ d88bd9df
...@@ -30,6 +30,7 @@ ...@@ -30,6 +30,7 @@
#include "third_party/blink/renderer/bindings/core/v8/node_or_string.h" #include "third_party/blink/renderer/bindings/core/v8/node_or_string.h"
#include "third_party/blink/renderer/core/css/css_selector.h" #include "third_party/blink/renderer/core/css/css_selector.h"
#include "third_party/blink/renderer/core/css/resolver/style_resolver.h" #include "third_party/blink/renderer/core/css/resolver/style_resolver.h"
#include "third_party/blink/renderer/core/css/style_change_reason.h"
#include "third_party/blink/renderer/core/css/style_engine.h" #include "third_party/blink/renderer/core/css/style_engine.h"
#include "third_party/blink/renderer/core/dom/attr.h" #include "third_party/blink/renderer/core/dom/attr.h"
#include "third_party/blink/renderer/core/dom/attribute.h" #include "third_party/blink/renderer/core/dom/attribute.h"
...@@ -2468,26 +2469,26 @@ void Node::DefaultEventHandler(Event* event) { ...@@ -2468,26 +2469,26 @@ void Node::DefaultEventHandler(Event* event) {
} }
void Node::WillCallDefaultEventHandler(const Event& event) { void Node::WillCallDefaultEventHandler(const Event& event) {
if (!IsFocused() || GetDocument().LastFocusType() != kWebFocusTypeMouse || if (!event.IsKeyboardEvent())
GetDocument().HadKeyboardEvent()) {
return; return;
}
if (!event.IsKeyboardEvent() || event.type() != EventTypeNames::keydown) { if (!IsFocused() || GetDocument().LastFocusType() != kWebFocusTypeMouse)
if (event.IsUIEvent())
GetDocument().SetHadKeyboardEvent(false);
return; return;
}
bool old_should_have_focus_appearance = ShouldHaveFocusAppearance(); if (event.type() != EventTypeNames::keydown ||
GetDocument().HadKeyboardEvent())
return;
GetDocument().SetHadKeyboardEvent(true); GetDocument().SetHadKeyboardEvent(true);
// Changes to HadKeyboardEvent may affect ShouldHaveFocusAppearance() and // Changes to HadKeyboardEvent may affect :focus-visible matching,
// LayoutTheme::IsFocused(). // ShouldHaveFocusAppearance and LayoutTheme::IsFocused().
// Inform LayoutTheme if ShouldHaveFocusAppearance() changes. // Inform LayoutTheme if HadKeyboardEvent changes.
if (old_should_have_focus_appearance != ShouldHaveFocusAppearance() && if (GetLayoutObject()) {
GetLayoutObject()) {
GetLayoutObject()->InvalidateIfControlStateChanged(kFocusControlState); GetLayoutObject()->InvalidateIfControlStateChanged(kFocusControlState);
if (RuntimeEnabledFeatures::CSSFocusVisibleEnabled() && IsContainerNode())
ToContainerNode(*this).FocusVisibleStateChanged();
} }
} }
...@@ -2579,6 +2580,8 @@ HTMLSlotElement* Node::assignedSlotForBinding() { ...@@ -2579,6 +2580,8 @@ HTMLSlotElement* Node::assignedSlotForBinding() {
} }
void Node::SetFocused(bool flag, WebFocusType focus_type) { void Node::SetFocused(bool flag, WebFocusType focus_type) {
if (!flag || focus_type == kWebFocusTypeMouse)
GetDocument().SetHadKeyboardEvent(false);
GetDocument().UserActionElements().SetFocused(this, flag); GetDocument().UserActionElements().SetFocused(this, flag);
} }
......
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