Skip to content

Commit

Permalink
[popover] event.preventDefault() should not cancel popover light dismiss
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=254384

Reviewed by Tim Nguyen.

Move light dismiss logic to PointerCaptureController::dispatchEvent so the event can't
be cancelled, also see:
whatwg/dom#1117 (comment)

* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/light-dismiss-event-ordering-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt:
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::handlePopoverLightDismiss):
* Source/WebCore/dom/Document.h:
* Source/WebCore/dom/Node.cpp:
(WebCore::Node::defaultEventHandler):
* Source/WebCore/page/PointerCaptureController.cpp:
(WebCore::PointerCaptureController::pointerEventWillBeDispatched):

Canonical link: https://commits.webkit.org/262283@main
  • Loading branch information
rwlbuis committed Mar 29, 2023
1 parent e537d9e commit afc536e
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 41 deletions.
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
target

PASS Tests the interactions between popover light dismiss and pointer/mouse events. eventName: pointerdown, capture: true
FAIL Tests the interactions between popover light dismiss and pointer/mouse events. eventName: pointerup, capture: true assert_equals: The popover should be closed via light dismiss even when preventDefault is called. expected 0 but got 1
PASS Tests the interactions between popover light dismiss and pointer/mouse events. eventName: pointerup, capture: true
PASS Tests the interactions between popover light dismiss and pointer/mouse events. eventName: mousedown, capture: true
PASS Tests the interactions between popover light dismiss and pointer/mouse events. eventName: mouseup, capture: true
PASS Tests the interactions between popover light dismiss and pointer/mouse events. eventName: click, capture: true
PASS Tests the interactions between popover light dismiss and pointer/mouse events. eventName: pointerdown, capture: false
FAIL Tests the interactions between popover light dismiss and pointer/mouse events. eventName: pointerup, capture: false assert_equals: The popover should be closed via light dismiss even when preventDefault is called. expected 0 but got 1
PASS Tests the interactions between popover light dismiss and pointer/mouse events. eventName: pointerup, capture: false
PASS Tests the interactions between popover light dismiss and pointer/mouse events. eventName: mousedown, capture: false
PASS Tests the interactions between popover light dismiss and pointer/mouse events. eventName: mouseup, capture: false
PASS Tests the interactions between popover light dismiss and pointer/mouse events. eventName: click, capture: false
FAIL Tests the order of pointer/mouse events during popover light dismiss. assert_array_equals: pointer and popover events should be fired in the correct order. expected property 2 to be "beforetoggle newState: closed" but got "pointerup" (expected array ["pointerdown", "mousedown", "beforetoggle newState: closed", "pointerup", "mouseup", "click"] got ["pointerdown", "mousedown", "pointerup", "beforetoggle newState: closed", "mouseup", "click"])
PASS Tests the order of pointer/mouse events during popover light dismiss.

Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
Popover 1 Popover 1 Popover1 anchor (no action) Outside all popovers Next control after popover1 Popover 3 - button 3 Popover 6 Popover8 anchor (no action) Open convoluted popover Popover 1

PASS Clicking outside a popover will dismiss the popover
FAIL Canceling pointer events should not keep clicks from light dismissing popovers assert_false: preventDefault should not prevent light dismiss expected false got true
FAIL Clicking inside a popover does not close that popover assert_false: expected false got true
FAIL Popovers close on pointerup, not pointerdown assert_false: expected false got true
FAIL Synthetic events can't close popovers assert_false: expected false got true
FAIL Moving focus outside the popover should not dismiss the popover promise_test: Unhandled rejection with value: object "InvalidStateError: Element has unexpected visibility state"
PASS Canceling pointer events should not keep clicks from light dismissing popovers
PASS Clicking inside a popover does not close that popover
PASS Popovers close on pointerup, not pointerdown
PASS Synthetic events can't close popovers
FAIL Moving focus outside the popover should not dismiss the popover assert_equals: Focus should move to a button outside the popover expected Element node <button id="after_p1">Next control after popover1</button> but got Element node <body><button id="b1t" popovertarget="p1">Popover 1</butt...
FAIL Clicking inside a child popover shouldn't close either popover promise_test: Unhandled rejection with value: object "InvalidStateError: Element has unexpected visibility state"
FAIL Clicking inside a parent popover should close child popover promise_test: Unhandled rejection with value: object "InvalidStateError: Element has unexpected visibility state"
PASS Clicking on invoking element, after using it for activation, shouldn't close its popover
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
Popover 1 Popover 1 Popover1 anchor (no action) Outside all popovers Next control after popover1 Popover 3 - button 3 Popover 6 Popover8 anchor (no action) Open convoluted popover Popover 1

PASS Clicking outside a popover will dismiss the popover
FAIL Canceling pointer events should not keep clicks from light dismissing popovers assert_false: preventDefault should not prevent light dismiss expected false got true
FAIL Clicking inside a popover does not close that popover assert_false: expected false got true
FAIL Popovers close on pointerup, not pointerdown assert_false: expected false got true
FAIL Synthetic events can't close popovers assert_false: expected false got true
FAIL Moving focus outside the popover should not dismiss the popover promise_test: Unhandled rejection with value: object "InvalidStateError: Element has unexpected visibility state"
FAIL Clicking inside a child popover shouldn't close either popover promise_test: Unhandled rejection with value: object "InvalidStateError: Element has unexpected visibility state"
FAIL Clicking inside a parent popover should close child popover promise_test: Unhandled rejection with value: object "InvalidStateError: Element has unexpected visibility state"
PASS Canceling pointer events should not keep clicks from light dismissing popovers
PASS Clicking inside a popover does not close that popover
PASS Popovers close on pointerup, not pointerdown
PASS Synthetic events can't close popovers
PASS Moving focus outside the popover should not dismiss the popover
PASS Clicking inside a child popover shouldn't close either popover
PASS Clicking inside a parent popover should close child popover
PASS Clicking on invoking element, after using it for activation, shouldn't close its popover
PASS Clicking on invoking element, after using it for activation, shouldn't close its popover (nested case)
PASS Clicking on invoking element, after using it for activation, shouldn't close its popover (nested case, not used for invocation)
Expand Down
3 changes: 1 addition & 2 deletions Source/WebCore/dom/Document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8983,7 +8983,7 @@ void Document::hideAllPopoversUntil(Element* endpoint, FocusPreviousElement focu
}

// https://html.spec.whatwg.org/#popover-light-dismiss
void Document::handlePopoverLightDismiss(PointerEvent& event)
void Document::handlePopoverLightDismiss(const PointerEvent& event, Node& target)
{
ASSERT(event.isTrusted());

Expand All @@ -8992,7 +8992,6 @@ void Document::handlePopoverLightDismiss(PointerEvent& event)
return;

RefPtr popoverToAvoidHiding = [&]() -> HTMLElement* {
auto& target = downcast<Node>(*event.target());
auto* startElement = is<Element>(target) ? &downcast<Element>(target) : target.parentElement();
auto [clickedPopover, invokerPopover] = [&]() {
RefPtr<HTMLElement> clickedPopover;
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/dom/Document.h
Original file line number Diff line number Diff line change
Expand Up @@ -1633,7 +1633,7 @@ class Document
HTMLElement* topmostAutoPopover() const;

void hideAllPopoversUntil(Element*, FocusPreviousElement, FireEvents);
void handlePopoverLightDismiss(PointerEvent&);
void handlePopoverLightDismiss(const PointerEvent&, Node&);

#if ENABLE(ATTACHMENT_ELEMENT)
void registerAttachmentIdentifier(const String&, const HTMLImageElement&);
Expand Down
3 changes: 0 additions & 3 deletions Source/WebCore/dom/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2498,9 +2498,6 @@ void Node::defaultEventHandler(Event& event)
}
}
#endif
} else if (eventType == eventNames.pointerdownEvent || eventType == eventNames.pointerupEvent) {
if (is<PointerEvent>(event))
document().handlePopoverLightDismiss(downcast<PointerEvent>(event));
} else if (eventNames.isWheelEventType(eventType) && is<WheelEvent>(event)) {
// If we don't have a renderer, send the wheel event to the first node we find with a renderer.
// This is needed for <option> and <optgroup> elements so that <select>s get a wheel scroll.
Expand Down
38 changes: 18 additions & 20 deletions Source/WebCore/page/PointerCaptureController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -386,30 +386,28 @@ void PointerCaptureController::pointerEventWillBeDispatched(const PointerEvent&

auto pointerId = event.pointerId();

auto& element = downcast<Element>(*target);
if (event.pointerType() != touchPointerEventType()) {
if (RefPtr capturingData = m_activePointerIdsToCapturingData.get(pointerId))
capturingData->pointerIsPressed = isPointerdown;
return;
} else if (isPointerdown) {
// https://w3c.github.io/pointerevents/#implicit-pointer-capture

// Some input devices (such as touchscreens) implement a "direct manipulation" metaphor where a pointer is intended to act primarily on the UI
// element it became active upon (providing a physical illusion of direct contact, instead of indirect contact via a cursor that conceptually
// floats above the UI). Such devices are identified by the InputDeviceCapabilities.pointerMovementScrolls property and should have "implicit
// pointer capture" behavior as follows.

// Direct manipulation devices should behave exactly as if setPointerCapture was called on the target element just before the invocation of any
// pointerdown listeners. The hasPointerCapture API may be used (eg. within any pointerdown listener) to determine whether this has occurred. If
// releasePointerCapture is not called for the pointer before the next pointer event is fired, then a gotpointercapture event will be dispatched
// to the target (as normal) indicating that capture is active.

auto capturingData = ensureCapturingDataForPointerEvent(event);
capturingData->pointerIsPressed = true;
setPointerCapture(&element, pointerId);
}

if (!isPointerdown)
return;

// https://w3c.github.io/pointerevents/#implicit-pointer-capture

// Some input devices (such as touchscreens) implement a "direct manipulation" metaphor where a pointer is intended to act primarily on the UI
// element it became active upon (providing a physical illusion of direct contact, instead of indirect contact via a cursor that conceptually
// floats above the UI). Such devices are identified by the InputDeviceCapabilities.pointerMovementScrolls property and should have "implicit
// pointer capture" behavior as follows.

// Direct manipulation devices should behave exactly as if setPointerCapture was called on the target element just before the invocation of any
// pointerdown listeners. The hasPointerCapture API may be used (eg. within any pointerdown listener) to determine whether this has occurred. If
// releasePointerCapture is not called for the pointer before the next pointer event is fired, then a gotpointercapture event will be dispatched
// to the target (as normal) indicating that capture is active.

auto capturingData = ensureCapturingDataForPointerEvent(event);
capturingData->pointerIsPressed = true;
setPointerCapture(downcast<Element>(target), pointerId);
element.document().handlePopoverLightDismiss(event, element);
}

auto PointerCaptureController::ensureCapturingDataForPointerEvent(const PointerEvent& event) -> Ref<CapturingData>
Expand Down

0 comments on commit afc536e

Please sign in to comment.