Changeset 271146 in webkit


Ignore:
Timestamp:
Jan 5, 2021 5:39:54 AM (19 months ago)
Author:
Manuel Rego Casasnovas
Message:

[selectors] :focus should match inside the focus event
​https://bugs.webkit.org/show_bug.cgi?id=220243

Reviewed by Frédéric Wang.

LayoutTests/imported/w3c:

Import focus-in-focus-event-001.html test from WPT.
Rebaseline 2 tests:

  • focus-autofocus.html now passes after fixing this bug.
  • focus-visible-005.html now fails, but it was passing because it was not checking anything due to this bug.
  • web-platform-tests/css/selectors/focus-in-focus-event-001-expected.txt: Added.
  • web-platform-tests/css/selectors/focus-in-focus-event-001.html: Added.
  • web-platform-tests/css/selectors/focus-visible-005-expected.txt: Rebaseline.
  • web-platform-tests/css/selectors/w3c-import.log:
  • web-platform-tests/html/semantics/selectors/pseudo-classes/focus-autofocus-expected.txt: Rebaesline.

Source/WebCore:

Before this patch this was working different in WebKit than Chromium and Firefox.
The specs are clear about the expected behavior for this:

A similar fix has been done on Blink in r201118 by <​kochi@chromium.org>
(​https://chromium.googlesource.com/chromium/src/+/cc49a6966f5ae6d89e86fbd3f53e4deea4e066ca).

Test: imported/w3c/web-platform-tests/css/selectors/focus-in-focus-event-001.html

  • dom/Document.cpp:

(WebCore::Document::setFocusedElement): Just moved the call to m_focusedElement->setFocus(true)
right before dispatching the events.

LayoutTests:

depend on the platform.
This issue has been fixed upstream in ​https://github.com/web-platform-tests/wpt/pull/27041
and these tests will be re-imported and unskipped in a follow-up patch.

Location:
trunk
Files:
2 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r271142 r271146  
     12021-01-05  Manuel Rego Casasnovas  <rego@igalia.com>
     2
     3        [selectors] :focus should match inside the focus event
     4        https://bugs.webkit.org/show_bug.cgi?id=220243
     5
     6        Reviewed by Frédéric Wang.
     7
     8        * TestExpectations: Skip some :focus-visible tests as current failure results
     9        depend on the platform.
     10        This issue has been fixed upstream in https://github.com/web-platform-tests/wpt/pull/27041
     11        and these tests will be re-imported and unskipped in a follow-up patch.
     12
    1132021-01-04  Rob Buis  <rbuis@igalia.com>
    214

  • trunk/LayoutTests/TestExpectations

    r271142 r271146  
    12311231imported/w3c/web-platform-tests/css/selectors/old-tests/css3-modsel-173a.xml [ ImageOnlyFailure ]
    12321232imported/w3c/web-platform-tests/css/selectors/old-tests/css3-modsel-173b.xml [ ImageOnlyFailure ]
     1233webkit.org/b/185859 imported/w3c/web-platform-tests/css/selectors/focus-visible-001.html [ Skip ]
    12331234webkit.org/b/185859 imported/w3c/web-platform-tests/css/selectors/focus-visible-002.html [ Skip ]
     1235webkit.org/b/185859 imported/w3c/web-platform-tests/css/selectors/focus-visible-007.html [ Skip ]
     1236webkit.org/b/185859 imported/w3c/web-platform-tests/css/selectors/focus-visible-009.html [ Skip ]
     1237webkit.org/b/185859 imported/w3c/web-platform-tests/css/selectors/focus-visible-010.html [ Skip ]
    12341238webkit.org/b/185859 imported/w3c/web-platform-tests/css/selectors/focus-visible-012.html [ Skip ]
    12351239webkit.org/b/217904 imported/w3c/web-platform-tests/css/selectors/is-where-visited.html [ ImageOnlyFailure ]

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r271142 r271146  
     12021-01-05  Manuel Rego Casasnovas  <rego@igalia.com>
     2
     3        [selectors] :focus should match inside the focus event
     4        https://bugs.webkit.org/show_bug.cgi?id=220243
     5
     6        Reviewed by Frédéric Wang.
     7
     8        Import focus-in-focus-event-001.html test from WPT.
     9        Rebaseline 2 tests:
     10        - focus-autofocus.html now passes after fixing this bug.
     11        - focus-visible-005.html now fails, but it was passing because it was not checking anything due to this bug.
     12
     13        * web-platform-tests/css/selectors/focus-in-focus-event-001-expected.txt: Added.
     14        * web-platform-tests/css/selectors/focus-in-focus-event-001.html: Added.
     15        * web-platform-tests/css/selectors/focus-visible-005-expected.txt: Rebaseline.
     16        * web-platform-tests/css/selectors/w3c-import.log:
     17        * web-platform-tests/html/semantics/selectors/pseudo-classes/focus-autofocus-expected.txt: Rebaesline.
     18
     19
    1202021-01-04  Rob Buis  <rbuis@igalia.com>
    221

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/selectors/focus-visible-005-expected.txt

    r267650 r271146  
    77I will be focused programmatically.
    88
    9 PASS Programmatic focus after click should not match :focus-visible
     9FAIL Programmatic focus after click should not match :focus-visible assert_equals: expected "none" but got "auto"
    1010

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/selectors/w3c-import.log

    r270933 r271146  
    2929/LayoutTests/imported/w3c/web-platform-tests/css/selectors/floating-first-letter-feff.html
    3030/LayoutTests/imported/w3c/web-platform-tests/css/selectors/focus-display-none-001.html
     31/LayoutTests/imported/w3c/web-platform-tests/css/selectors/focus-in-focus-event-001.html
    3132/LayoutTests/imported/w3c/web-platform-tests/css/selectors/focus-visible-001.html
    3233/LayoutTests/imported/w3c/web-platform-tests/css/selectors/focus-visible-002.html

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/selectors/pseudo-classes/focus-autofocus-expected.txt

    r262254 r271146  
    11
    22
    3 FAIL :focus selector should work with an autofocused element. assert_array_equals: lengths differ, expected array [Element node <input autofocus=""></input>] length 1, got object "[object NodeList]" length 0
     3PASS :focus selector should work with an autofocused element.
    44

  • trunk/Source/WebCore/ChangeLog

    r271137 r271146  
     12021-01-05  Manuel Rego Casasnovas  <rego@igalia.com>
     2
     3        [selectors] :focus should match inside the focus event
     4        https://bugs.webkit.org/show_bug.cgi?id=220243
     5
     6        Reviewed by Frédéric Wang.
     7
     8        Before this patch this was working different in WebKit than Chromium and Firefox.
     9        The specs are clear about the expected behavior for this:
     10        - From https://drafts.csswg.org/selectors-4/#the-focus-pseudo:
     11          "The :focus pseudo-class applies while an element has the focus"
     12        - From https://w3c.github.io/uievents/#event-type-focus:
     13          "The focus MUST be given to the element before the dispatch of this event type"
     14
     15        A similar fix has been done on Blink in r201118 by <kochi@chromium.org>
     16        (https://chromium.googlesource.com/chromium/src/+/cc49a6966f5ae6d89e86fbd3f53e4deea4e066ca).
     17
     18        Test: imported/w3c/web-platform-tests/css/selectors/focus-in-focus-event-001.html
     19
     20        * dom/Document.cpp:
     21        (WebCore::Document::setFocusedElement): Just moved the call to m_focusedElement->setFocus(true)
     22        right before dispatching the events.
     23
    1242021-01-04  Alexey Shvayka  <shvaikalesh@gmail.com>
    225

  • trunk/Source/WebCore/dom/Document.cpp

    r271125 r271146  
    44464446        m_focusedElement = newFocusedElement;
    44474447        setFocusNavigationStartingNode(m_focusedElement.get());
    4448 
    4449         // Dispatch the focus event and let the node do any other focus related activities (important for text fields)
    4450         m_focusedElement->dispatchFocusEvent(oldFocusedElement.copyRef(), direction);
    4451 
     4448        m_focusedElement->setFocus(true);
     4449
     4450        // The setFocus call triggers a blur and a focus event. Event handlers could cause the focused element to be cleared.
    44524451        if (m_focusedElement != newFocusedElement) {
    44534452            // handler shifted focus
    … …  
    44554454        }
    44564455
    4457         m_focusedElement->dispatchFocusInEvent(eventNames().focusinEvent, oldFocusedElement.copyRef()); // DOM level 3 bubbling focus event.
     4456        // Dispatch the focus event and let the node do any other focus related activities (important for text fields)
     4457        m_focusedElement->dispatchFocusEvent(oldFocusedElement.copyRef(), direction);
    44584458
    44594459        if (m_focusedElement != newFocusedElement) {
    … …  
    44624462        }
    44634463
    4464         // FIXME: We should remove firing DOMFocusInEvent event when we are sure no content depends
    4465         // on it, probably when <rdar://problem/8503958> is m.
    4466         m_focusedElement->dispatchFocusInEvent(eventNames().DOMFocusInEvent, oldFocusedElement.copyRef()); // DOM level 2 for compatibility.
     4464        m_focusedElement->dispatchFocusInEvent(eventNames().focusinEvent, oldFocusedElement.copyRef()); // DOM level 3 bubbling focus event.
    44674465
    44684466        if (m_focusedElement != newFocusedElement) {
    … …  
    44714469        }
    44724470
    4473         m_focusedElement->setFocus(true);
    4474 
    4475         // The setFocus call triggers a blur and a focus event. Event handlers could cause the focused element to be cleared.
     4471        // FIXME: We should remove firing DOMFocusInEvent event when we are sure no content depends
     4472        // on it, probably when <rdar://problem/8503958> is m.
     4473        m_focusedElement->dispatchFocusInEvent(eventNames().DOMFocusInEvent, oldFocusedElement.copyRef()); // DOM level 2 for compatibility.
     4474
    44764475        if (m_focusedElement != newFocusedElement) {
    44774476            // handler shifted focus
Note: See TracChangeset for help on using the changeset viewer.