Diff
Modified: trunk/LayoutTests/ChangeLog (276697 => 276698)
--- trunk/LayoutTests/ChangeLog 2021-04-28 08:39:01 UTC (rev 276697)
+++ trunk/LayoutTests/ChangeLog 2021-04-28 08:51:37 UTC (rev 276698)
@@ -1,3 +1,12 @@
+2021-04-28 Manuel Rego Casasnovas <[email protected]>
+
+ [selectors] Using a modifier key on an element makes it stop matching :focus-visible
+ https://bugs.webkit.org/show_bug.cgi?id=225075
+
+ Reviewed by Ryosuke Niwa.
+
+ * platform/ios/TestExpectations: Skip new test.
+
2021-04-28 Cameron McCormack <[email protected]>
Fix some test typos.
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (276697 => 276698)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2021-04-28 08:39:01 UTC (rev 276697)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2021-04-28 08:51:37 UTC (rev 276698)
@@ -1,3 +1,13 @@
+2021-04-28 Manuel Rego Casasnovas <[email protected]>
+
+ [selectors] Using a modifier key on an element makes it stop matching :focus-visible
+ https://bugs.webkit.org/show_bug.cgi?id=225075
+
+ Reviewed by Ryosuke Niwa.
+
+ * web-platform-tests/css/selectors/focus-visible-021-expected.txt: Added.
+ * web-platform-tests/css/selectors/focus-visible-021.html: Added.
+
2021-04-28 Cameron McCormack <[email protected]>
Make img elements perform source selection even when not in the document.
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/css/selectors/focus-visible-021-expected.txt (0 => 276698)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/selectors/focus-visible-021-expected.txt (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/selectors/focus-visible-021-expected.txt 2021-04-28 08:51:37 UTC (rev 276698)
@@ -0,0 +1,9 @@
+This test checks that using a modifier key does not make the element to stop matching :focus-visible.
+If the element that says "Focused" has a red background, then the test result is a FAILURE.
+Press the Ctrl key.
+If the element keeps the green outline and has not a red background, then the test result is SUCCESS.
+Focused
+
+PASS ":focus-visible" should be a valid selector
+PASS Ctrl key doesn't make an element to stop matching :focus-visible
+
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/css/selectors/focus-visible-021.html (0 => 276698)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/selectors/focus-visible-021.html (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/selectors/focus-visible-021.html 2021-04-28 08:51:37 UTC (rev 276698)
@@ -0,0 +1,67 @@
+<!DOCTYPE html>
+<meta charset="utf-8" />
+<title>CSS Test (Selectors): Modifier key doesn't make element to stop matching :focus-visible</title>
+<link rel="author" title="Manuel Rego Casasnovas" href=""
+<link rel="help" href="" />
+<script src=""
+<script src=""
+<script src=""
+<script src=""
+<script src=""
+<style>
+ #warning {
+ display: none;
+ background: red;
+ }
+
+ @supports not selector(:focus-visible) {
+ #instructions {
+ display: none;
+ }
+
+ #warning {
+ display: block;
+ }
+ }
+
+ :focus-visible {
+ outline: solid thick green;
+ }
+
+ :focus:not(:focus-visible) {
+ outline: none;
+ background-color: red;
+ }
+</style>
+
+This test checks that using a modifier key does not make the element to stop matching <code>:focus-visible</code>.
+<ol id="instructions">
+ <li>If the element that says "Focused" has a red background, then the test result is a FAILURE.</li>
+ <li>Press the Ctrl key.</li>
+ <li>If the element keeps the green outline and has not a red background, then the test result is SUCCESS.</li>
+</ol>
+<p id="warning">Your user-agent does not support <code>:focus-visible</code> pseudo-class, please SKIP this test.</p>
+
+<div id="target" tabindex="0">Focused</div>
+<script>
+ // Check that :focus-visible is supported.
+ test_valid_selector(':focus-visible');
+
+ async_test(function(t) {
+ target.addEventListener("focus", t.step_func(() => {
+ assert_equals(getComputedStyle(target).outlineColor, "rgb(0, 128, 0)", `outlineColor for ${target.tagName}#${target.id} should be green`);
+ assert_not_equals(getComputedStyle(target).backgroundColor, "rgb(255, 0, 0)", `backgroundColor for ${target.tagName}#${target.id} should NOT be red`);
+
+ const ctrl_key = '\uE009';
+ test_driver.send_keys(target, ctrl_key);
+ }));
+
+ target.addEventListener("keyup", t.step_func_done(() => {
+ assert_equals(getComputedStyle(target).outlineColor, "rgb(0, 128, 0)", `outlineColor for ${target.tagName}#${target.id} should be green`);
+ assert_not_equals(getComputedStyle(target).backgroundColor, "rgb(255, 0, 0)", `backgroundColor for ${target.tagName}#${target.id} should NOT be red`);
+ }));
+
+ }, "Ctrl key doesn't make an element to stop matching :focus-visible");
+
+ target.focus();
+</script>
Modified: trunk/LayoutTests/platform/ios/TestExpectations (276697 => 276698)
--- trunk/LayoutTests/platform/ios/TestExpectations 2021-04-28 08:39:01 UTC (rev 276697)
+++ trunk/LayoutTests/platform/ios/TestExpectations 2021-04-28 08:51:37 UTC (rev 276698)
@@ -3282,6 +3282,7 @@
webkit.org/b/209734 imported/w3c/web-platform-tests/css/selectors/focus-visible-012.html [ Skip ]
webkit.org/b/209734 imported/w3c/web-platform-tests/css/selectors/focus-visible-013.html [ Skip ]
webkit.org/b/209734 imported/w3c/web-platform-tests/css/selectors/focus-visible-019.html [ Skip ]
+webkit.org/b/209734 imported/w3c/web-platform-tests/css/selectors/focus-visible-021.html [ Skip ]
webkit.org/b/209734 imported/w3c/web-platform-tests/css/selectors/focus-visible-script-focus-004.html [ Skip ]
webkit.org/b/209734 imported/w3c/web-platform-tests/css/selectors/focus-visible-script-focus-005.html [ Skip ]
webkit.org/b/209734 imported/w3c/web-platform-tests/css/selectors/focus-visible-script-focus-012.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (276697 => 276698)
--- trunk/Source/WebCore/ChangeLog 2021-04-28 08:39:01 UTC (rev 276697)
+++ trunk/Source/WebCore/ChangeLog 2021-04-28 08:51:37 UTC (rev 276698)
@@ -1,3 +1,28 @@
+2021-04-28 Manuel Rego Casasnovas <[email protected]>
+
+ [selectors] Using a modifier key on an element makes it stop matching :focus-visible
+ https://bugs.webkit.org/show_bug.cgi?id=225075
+
+ Reviewed by Ryosuke Niwa.
+
+ When we used a modifier key on a element that was matching :focus-visible, we stopped matching :focus-visible.
+ That was wrong, we shouldn't just start matching :focus-visible when a modifier key is used but not the other way around.
+
+ This patches fixed that without doing any work if the element is already matching :focus-visible when the user type a key.
+
+ Test: imported/w3c/web-platform-tests/css/selectors/focus-visible-021.html
+
+ * dom/Element.cpp:
+ (WebCore::shouldAlwaysHaveFocusVisibleWhenFocused): Helper method for inputs and content editable elements.
+ (WebCore::Element::setFocus): Use the new helper method.
+ (WebCore::Element::setHasFocusVisible): Add asserts to avoid setting/unsetting :focus-visible flag wrongly.
+ * dom/Node.cpp:
+ (WebCore::Node::isContentEditable const): Just mark as const.
+ (WebCore::Node::isContentRichlyEditable const): Ditto.
+ * dom/Node.h: Ditto.
+ * page/EventHandler.cpp:
+ (WebCore::EventHandler::internalKeyEvent): Don't do anything regarding :focus-visible flag if the element already matches :focus-visible.
+
2021-04-28 Cameron McCormack <[email protected]>
Make img elements perform source selection even when not in the document.
Modified: trunk/Source/WebCore/dom/Element.cpp (276697 => 276698)
--- trunk/Source/WebCore/dom/Element.cpp 2021-04-28 08:39:01 UTC (rev 276697)
+++ trunk/Source/WebCore/dom/Element.cpp 2021-04-28 08:51:37 UTC (rev 276698)
@@ -745,6 +745,11 @@
}
}
+static bool shouldAlwaysHaveFocusVisibleWhenFocused(const Element& element)
+{
+ return element.isTextField() || element.isContentEditable();
+}
+
void Element::setFocus(bool flag, FocusVisibility visibility)
{
if (flag == focused())
@@ -764,8 +769,7 @@
for (auto* element = this; element; element = element->parentElementInComposedTree())
element->setHasFocusWithin(flag);
- // Elements that support keyboard input (form inputs and contenteditable) always match :focus-visible when focused.
- setHasFocusVisible(flag && (visibility == FocusVisibility::Visible || isTextField() || isContentEditable()));
+ setHasFocusVisible(flag && (visibility == FocusVisibility::Visible || shouldAlwaysHaveFocusVisibleWhenFocused(*this)));
}
void Element::setHasFocusVisible(bool flag)
@@ -773,6 +777,11 @@
if (!document().settings().focusVisibleEnabled())
return;
+#if ASSERT_ENABLED
+ ASSERT(!flag || focused());
+ ASSERT(!focused() || !shouldAlwaysHaveFocusVisibleWhenFocused(*this) || flag);
+#endif
+
if (hasFocusVisible() == flag)
return;
Modified: trunk/Source/WebCore/dom/Node.cpp (276697 => 276698)
--- trunk/Source/WebCore/dom/Node.cpp 2021-04-28 08:39:01 UTC (rev 276697)
+++ trunk/Source/WebCore/dom/Node.cpp 2021-04-28 08:51:37 UTC (rev 276698)
@@ -732,12 +732,12 @@
return nullAtom();
}
-bool Node::isContentEditable()
+bool Node::isContentEditable() const
{
return computeEditability(UserSelectAllDoesNotAffectEditability, ShouldUpdateStyle::Update) != Editability::ReadOnly;
}
-bool Node::isContentRichlyEditable()
+bool Node::isContentRichlyEditable() const
{
return computeEditability(UserSelectAllIsAlwaysNonEditable, ShouldUpdateStyle::Update) == Editability::CanEditRichly;
}
Modified: trunk/Source/WebCore/dom/Node.h (276697 => 276698)
--- trunk/Source/WebCore/dom/Node.h 2021-04-28 08:39:01 UTC (rev 276697)
+++ trunk/Source/WebCore/dom/Node.h 2021-04-28 08:51:37 UTC (rev 276698)
@@ -317,8 +317,8 @@
bool hasEventTargetData() const { return hasNodeFlag(NodeFlag::HasEventTargetData); }
void setHasEventTargetData(bool flag) { setNodeFlag(NodeFlag::HasEventTargetData, flag); }
- WEBCORE_EXPORT bool isContentEditable();
- bool isContentRichlyEditable();
+ WEBCORE_EXPORT bool isContentEditable() const;
+ bool isContentRichlyEditable() const;
WEBCORE_EXPORT void inspect();
Modified: trunk/Source/WebCore/page/EventHandler.cpp (276697 => 276698)
--- trunk/Source/WebCore/page/EventHandler.cpp 2021-04-28 08:39:01 UTC (rev 276697)
+++ trunk/Source/WebCore/page/EventHandler.cpp 2021-04-28 08:51:37 UTC (rev 276698)
@@ -3543,16 +3543,15 @@
keydown->preventDefault();
keydown->setTarget(element);
- auto shouldMatchFocusVisible = [initialKeyEvent, keydown](const Element& element) {
- if (!element.focused())
- return false;
-
+ auto setHasFocusVisibleIfNeeded = [initialKeyEvent, keydown](Element& element) {
// If the user interacts with the page via the keyboard, the currently focused element should match :focus-visible.
// Just typing a modifier key is not considered user interaction with the page, but Shift + a (or Caps Lock + a) is considered an interaction.
- return keydown->modifierKeys().isEmpty() || ((keydown->shiftKey() || keydown->capsLockKey()) && !initialKeyEvent.text().isEmpty());
+ bool userHasInteractedViaKeyword = keydown->modifierKeys().isEmpty() || ((keydown->shiftKey() || keydown->capsLockKey()) && !initialKeyEvent.text().isEmpty());
+
+ if (element.focused() && userHasInteractedViaKeyword)
+ element.setHasFocusVisible(true);
};
- // FIXME: This is wrong for text form controls and contenteditable elements (https://webkit.org/b/225075).
- element->setHasFocusVisible(shouldMatchFocusVisible(*element));
+ setHasFocusVisibleIfNeeded(*element);
if (initialKeyEvent.type() == PlatformEvent::RawKeyDown) {
element->dispatchEvent(keydown);
@@ -3599,12 +3598,10 @@
// Focus may have changed during keydown handling, so refetch element.
// But if we are dispatching a fake backward compatibility keypress, then we pretend that the keypress happened on the original element.
if (!keydownResult) {
- element->setHasFocusVisible(false);
element = eventTargetElementForDocument(m_frame.document());
if (!element)
return false;
- // FIXME: This is wrong for text form controls and contenteditable elements (https://webkit.org/b/225075).
- element->setHasFocusVisible(shouldMatchFocusVisible(*element));
+ setHasFocusVisibleIfNeeded(*element);
}
PlatformKeyboardEvent keyPressEvent = initialKeyEvent;
