Skip to content

Commit

Permalink
Videos autoplay with sound on cnn.com pages after refresh in Safari
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=258696
rdar://110343800

Reviewed by Chris Dumez.

The HTML specification defines a very narrow set of gesture events
which will cause activation. WebKit currently triggers activation
for all uses of UserGestureIndicator, which leads to things like
"LeftCmd Up" after a Cmd-R to trigger playback during a reload.

Add a set of helper methods `userGestureTypeForPlatformEvent()` to
return the correct UserGestureType for a particular PlatformEvent.

Update UserGestureIndicator to only set the activation timestamp of
the window when passed a UserGestureType::ActivationTriggering.

Make UserGestureType::ActivationTriggering the default to handle
the currently large set of call sites that expect a user gesture
to be triggered from a programmatic call.

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/html/user-activation/activation-trigger-keyboard-enter-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/user-activation/activation-trigger-keyboard-escape-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/user-activation/activation-trigger-mouse-left-expected.txt:
* Source/WebCore/dom/UserGestureIndicator.cpp:
* Source/WebCore/dom/UserGestureIndicator.h:
* Source/WebCore/page/EventHandler.cpp:
(WebCore::userGestureTypeForPlatformEvent):
(WebCore::EventHandler::handleMousePressEvent):
(WebCore::EventHandler::handleMouseDoubleClickEvent):
(WebCore::EventHandler::handleMouseReleaseEvent):
(WebCore::EventHandler::internalKeyEvent):
(WebCore::EventHandler::handleTouchEvent):

Canonical link: https://commits.webkit.org/265688@main
  • Loading branch information
jernoble authored and eric-carlson committed Jul 1, 2023
1 parent 4498c48 commit f4cdea7
Show file tree
Hide file tree
Showing 17 changed files with 170 additions and 24 deletions.
1 change: 1 addition & 0 deletions LayoutTests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,7 @@ imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/mo
imported/w3c/web-platform-tests/html/user-activation/activation-trigger-keyboard-enter.html [ Skip ]
imported/w3c/web-platform-tests/html/user-activation/activation-trigger-keyboard-escape.html [ Skip ]
imported/w3c/web-platform-tests/html/user-activation/activation-trigger-mouse-left.html [ Skip ]
imported/w3c/web-platform-tests/html/user-activation/activation-trigger-mouse-right.html [ Skip ]
imported/w3c/web-platform-tests/html/webappapis/scripting/processing-model-2/unhandled-promise-rejections/allow-crossorigin.html [ Skip ]
imported/w3c/web-platform-tests/html/webappapis/scripting/processing-model-2/unhandled-promise-rejections/disallow-crossorigin.html [ Skip ]
imported/w3c/web-platform-tests/html/webappapis/scripting/processing-model-2/unhandled-promise-rejections/promise-rejection-events.html [ Skip ]
Expand Down
30 changes: 30 additions & 0 deletions LayoutTests/fast/html/transient-activation-expected.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
Tests that transient activation is generated by correct events.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS successfullyParsed is true

TEST COMPLETE
eventSender.mouseMoveTo(50, 50)
PASS EVENT(mousemove)
PASS navigator.userActivation.isActive is false
eventSender.mouseDown()
PASS EVENT(mousedown)
PASS navigator.userActivation.isActive is true
eventSender.mouseUp()
PASS EVENT(mouseup)
PASS navigator.userActivation.isActive is false
UIHelper.rawKeyDown('escape')
PASS EVENT(keydown)
PASS navigator.userActivation.isActive is false
UIHelper.rawKeyUp('esc')
PASS EVENT(keyup)
PASS navigator.userActivation.isActive is false
UIHelper.rawKeyDown('a')
PASS EVENT(keydown)
PASS navigator.userActivation.isActive is true
UIHelper.rawKeyUp('a')
PASS EVENT(keyup)
PASS navigator.userActivation.isActive is false

81 changes: 81 additions & 0 deletions LayoutTests/fast/html/transient-activation.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<script src="../../resources/js-test-pre.js"></script>
<script src="../../resources/ui-helper.js"></script>
<script>
description("Tests that transient activation is generated by correct events.");

testRunner.waitUntilDone();

function waitFor(element, type) {
return new Promise(resolve => {
element.addEventListener(type, event => {
testPassed(`EVENT(${event.type})`);
resolve(event);
}, { once: true });
});
}

async function doTest() {
internals.consumeTransientActivation();

let mouseMovePromise = waitFor(document, 'mousemove');
evalAndLog("eventSender.mouseMoveTo(50, 50)");
await mouseMovePromise;
shouldBeFalse("navigator.userActivation.isActive");
internals.consumeTransientActivation();

let mouseDownPromise = waitFor(document, 'mousedown');
evalAndLog("eventSender.mouseDown()");
await mouseDownPromise;
shouldBeTrue("navigator.userActivation.isActive");
internals.consumeTransientActivation();

let mouseUpPromise = waitFor(document, 'mouseup');
evalAndLog("eventSender.mouseUp()");
await mouseUpPromise;
shouldBeFalse("navigator.userActivation.isActive");
internals.consumeTransientActivation();

var keyDownPromise = waitFor(document, 'keydown');
var promise = evalAndLog("UIHelper.rawKeyDown('escape')");
await Promise.all([promise, keyDownPromise]);
shouldBeFalse("navigator.userActivation.isActive");
internals.consumeTransientActivation();

var keyUpPromise = waitFor(document, 'keyup');
var promise = evalAndLog("UIHelper.rawKeyUp('esc')");
await Promise.all([promise, keyUpPromise]);
shouldBeFalse("navigator.userActivation.isActive");
internals.consumeTransientActivation();

keyDownPromise = waitFor(document, 'keydown');
var promise = evalAndLog("UIHelper.rawKeyDown('a')");
await Promise.all([promise, keyDownPromise]);
shouldBeTrue("navigator.userActivation.isActive");
internals.consumeTransientActivation();

keyUpPromise = waitFor(document, 'keyup');
var promise = evalAndLog("UIHelper.rawKeyUp('a')");
await Promise.all([promise, keyUpPromise]);
shouldBeFalse("navigator.userActivation.isActive");
internals.consumeTransientActivation();
}

window.addEventListener("load", event => {
doTest().catch(e => {
testFailed(e.description);
}).finally(() => {
testRunner.notifyDone();
});
});
</script>
</head>
<body>
<button id="target" style="width: 100px; height: 100px;"></button>
<script src="../../resources/js-test-post.js"></script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
CONSOLE MESSAGE: The Escape key may not be used as a user gesture to enter fullscreen
Tests that pressing the Escape key cannot be used as user interaction to enter fullscreen.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,5 @@ Tests user activation from a ENTER keyboard event.

Press ENTER key.

Harness Error (TIMEOUT), message = null

TIMEOUT Activation through ENTER keyboard event Test timed out
PASS Activation through ENTER keyboard event

Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,5 @@ Tests missing user activation from a ESCAPE keyboard event.

Press ESCAPE key.

Harness Error (TIMEOUT), message = null

TIMEOUT Activation through ESCAPE keyboard event Test timed out
PASS Activation through ESCAPE keyboard event

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,5 @@ Tests user activation from a mouse click.

Click anywhere in the document.

Harness Error (TIMEOUT), message = null

TIMEOUT Activation through left-click mouse event Test timed out
PASS Activation through left-click mouse event

Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ Tests user activation from a pointer click.

Click anywhere in the document.

FAIL Activation through mouse pointerevent click assert_false: mouse pointerup should have no activation after pointerdown consumption expected false got true
PASS Activation through mouse pointerevent click

Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ Tests that pressing/releasing 'Escape' key is not treated as a user activation.



FAIL 'Escape' key doesn't activate a page. assert_false: No user activation on keydown expected false got true
PASS 'Escape' key doesn't activate a page.

1 change: 1 addition & 0 deletions LayoutTests/platform/gtk/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -1789,6 +1789,7 @@ webkit.org/b/100238 fast/history/window-open.html [ Failure ]

# These failures appear to be bugs in the EventSender
webkit.org/b/53964 fast/forms/listbox-onchange.html [ Failure ]
fast/html/transient-activation.html [ Skip ]

Bug(GTK) http/tests/misc/acid3.html [ Failure ]

Expand Down
1 change: 0 additions & 1 deletion LayoutTests/platform/ios-wk2/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -1175,7 +1175,6 @@ webkit.org/b/161631 imported/w3c/web-platform-tests/html/browsers/browsing-the-w

# Newly imported WPT tests that are timing out on iOS.
imported/w3c/web-platform-tests/html/semantics/forms/the-button-element/button-activate-keyup-prevented.html [ Skip ]
imported/w3c/web-platform-tests/html/user-activation/activation-trigger-mouse-right.html [ Skip ]
imported/w3c/web-platform-tests/uievents/click/auxclick_event.html [ Skip ]
imported/w3c/web-platform-tests/uievents/click/click_event_target_child_parent.html [ Skip ]
imported/w3c/web-platform-tests/uievents/click/click_event_target_siblings.html [ Skip ]
Expand Down
1 change: 1 addition & 0 deletions LayoutTests/platform/ios/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -1037,6 +1037,7 @@ fast/forms/validation-message-maxLength.html [ Skip ]
fast/forms/ValidityState-valueMissing-002.html [ Skip ]
fast/frames/focus-controller-crash-change-event.html
fast/html/details-keyboard-show-hide.html [ Skip ]
fast/html/transient-activation.html [ Skip ]
fast/shadow-dom/activate-over-slotted-content.html [ Skip ]
fast/shadow-dom/clear-active-state-in-shadow.html [ Skip ]
fast/shadow-dom/focus-across-details-element.html [ Skip ]
Expand Down
2 changes: 1 addition & 1 deletion LayoutTests/platform/mac-wk1/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -2352,7 +2352,7 @@ imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_
imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_pointercapture_in_frame.html [ Pass Failure ]
imported/w3c/web-platform-tests/pointerevents/pointerevent_pointercapture-in-custom-element.html [ Pass Failure ]

webkit.org/b/236128 imported/w3c/web-platform-tests/html/user-activation/activation-trigger-mouse-right.html [ Skip ]
webkit.org/b/236128 fullscreen/full-screen-iframe-with-max-width-height.html [ Skip ]

imported/w3c/web-platform-tests/html/semantics/embedded-content/the-object-element/object-param-url.html [ ImageOnlyFailure ]

Expand Down
2 changes: 2 additions & 0 deletions LayoutTests/platform/mac/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -2722,3 +2722,5 @@ webkit.org/b/258181 [ Monterey+ Debug ] inspector/debugger/async-stack-trace-tru
# webkit.org/b/258328 Umbrella Bug: Batch mark expectations slowing down EWS
[ arm64 ] imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audioworklet-interface/audioworklet-denormals.https.window.html [ Failure ]
[ Debug ] media/modern-media-controls/pip-support/pip-support-enabled.html [ Pass Crash ]

webkit.org/b/236128 imported/w3c/web-platform-tests/html/user-activation/activation-trigger-mouse-right.html [ Skip ]
6 changes: 5 additions & 1 deletion Source/WebCore/dom/UserGestureIndicator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,11 @@ UserGestureIndicator::UserGestureIndicator(std::optional<ProcessingUserGestureSt
}
}

if (auto* window = document->domWindow())
// https://html.spec.whatwg.org/multipage/interaction.html#user-activation-processing-model
// When a user interaction causes firing of an activation triggering input event in a Document...
// NOTE: Only activate the relevent DOMWindow when the gestureType is an ActivationTriggering one
auto* window = document->domWindow();
if (window && gestureType == UserGestureType::ActivationTriggering)
window->notifyActivated(currentToken()->startTime());
}
}
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/dom/UserGestureIndicator.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ enum ProcessingUserGestureState {
NotProcessingUserGesture
};

enum class UserGestureType { EscapeKey, Other };
enum class UserGestureType { EscapeKey, ActivationTriggering, Other };

class UserGestureToken : public RefCounted<UserGestureToken>, public CanMakeWeakPtr<UserGestureToken> {
public:
Expand Down Expand Up @@ -135,7 +135,7 @@ class UserGestureIndicator {

// If a document is provided, its last known user gesture timestamp is updated.
enum class ProcessInteractionStyle { Immediate, Delayed };
WEBCORE_EXPORT explicit UserGestureIndicator(std::optional<ProcessingUserGestureState>, Document* = nullptr, UserGestureType = UserGestureType::Other, ProcessInteractionStyle = ProcessInteractionStyle::Immediate, std::optional<UUID> authorizationToken = std::nullopt);
WEBCORE_EXPORT explicit UserGestureIndicator(std::optional<ProcessingUserGestureState>, Document* = nullptr, UserGestureType = UserGestureType::ActivationTriggering, ProcessInteractionStyle = ProcessInteractionStyle::Immediate, std::optional<UUID> authorizationToken = std::nullopt);
WEBCORE_EXPORT explicit UserGestureIndicator(RefPtr<UserGestureToken>, UserGestureToken::GestureScope = UserGestureToken::GestureScope::All, UserGestureToken::IsPropagatedFromFetch = UserGestureToken::IsPropagatedFromFetch::No);
WEBCORE_EXPORT ~UserGestureIndicator();

Expand Down
48 changes: 41 additions & 7 deletions Source/WebCore/page/EventHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,42 @@ class MaximumDurationTracker {
MonotonicTime m_start;
};

static UserGestureType userGestureTypeForPlatformEvent(const PlatformKeyboardEvent& keyEvent)
{
// https://html.spec.whatwg.org/multipage/interaction.html#activation-triggering-input-event
// An activation triggering input event is any event whose isTrusted attribute is true and whose type is one of:
// * "keydown", provided the key is neither the Esc key nor a shortcut key reserved by the user agent.
if (keyEvent.windowsVirtualKeyCode() == VK_ESCAPE)
return UserGestureType::EscapeKey;
if (keyEvent.type() == PlatformEventType::KeyDown)
return UserGestureType::ActivationTriggering;

// FIXME: This check does not yet handle whether the event represents a "shortcut key reserved by the user agent".
return UserGestureType::Other;
}

static UserGestureType userGestureTypeForPlatformEvent(const PlatformMouseEvent& mouseEvent)
{
// ...
// * "mousedown".
// * "pointerdown", provided the event's pointerType is "mouse".
if (mouseEvent.type() == PlatformEventType::MousePressed)
return UserGestureType::ActivationTriggering;
return UserGestureType::Other;
}

#if ENABLE(TOUCH_EVENTS) && !ENABLE(IOS_TOUCH_EVENTS)
static UserGestureType userGestureTypeForPlatformEvent(const PlatformTouchEvent& touchEvent)
{
// ...
// * "pointerup", provided the event's pointerType is not "mouse".
// * "touchend".
if (touchEvent.type() == PlatformEventType::TouchEnd)
return UserGestureType::ActivationTriggering;
return UserGestureType::Other;
}
#endif

#if ENABLE(TOUCH_EVENTS) && !ENABLE(IOS_TOUCH_EVENTS)
class SyntheticTouchPoint : public PlatformTouchPoint {
public:
Expand Down Expand Up @@ -1741,7 +1777,7 @@ bool EventHandler::handleMousePressEvent(const PlatformMouseEvent& platformMouse
return true;
#endif

UserGestureIndicator gestureIndicator(ProcessingUserGesture, m_frame.document());
UserGestureIndicator gestureIndicator(ProcessingUserGesture, m_frame.document(), userGestureTypeForPlatformEvent(platformMouseEvent));

// FIXME (bug 68185): this call should be made at another abstraction layer
m_frame.loader().resetMultipleFormSubmissionProtection();
Expand Down Expand Up @@ -1876,7 +1912,7 @@ bool EventHandler::handleMouseDoubleClickEvent(const PlatformMouseEvent& platfor

m_frame.selection().setCaretBlinkingSuspended(false);

UserGestureIndicator gestureIndicator(ProcessingUserGesture, m_frame.document());
UserGestureIndicator gestureIndicator(ProcessingUserGesture, m_frame.document(), userGestureTypeForPlatformEvent(platformMouseEvent));

#if ENABLE(POINTER_LOCK)
if (m_frame.page()->pointerLockController().isLocked()) {
Expand Down Expand Up @@ -2181,7 +2217,7 @@ bool EventHandler::handleMouseReleaseEvent(const PlatformMouseEvent& platformMou
return true;
#endif

UserGestureIndicator gestureIndicator(ProcessingUserGesture, m_frame.document(), UserGestureType::Other, UserGestureIndicator::ProcessInteractionStyle::Immediate, platformMouseEvent.authorizationToken());
UserGestureIndicator gestureIndicator(ProcessingUserGesture, m_frame.document(), userGestureTypeForPlatformEvent(platformMouseEvent), UserGestureIndicator::ProcessInteractionStyle::Immediate, platformMouseEvent.authorizationToken());

#if ENABLE(PAN_SCROLLING)
m_autoscrollController->handleMouseReleaseEvent(platformMouseEvent);
Expand Down Expand Up @@ -3684,9 +3720,7 @@ bool EventHandler::internalKeyEvent(const PlatformKeyboardEvent& initialKeyEvent
if (!element)
return false;

UserGestureType gestureType = UserGestureType::Other;
if (initialKeyEvent.windowsVirtualKeyCode() == VK_ESCAPE)
gestureType = UserGestureType::EscapeKey;
UserGestureType gestureType = userGestureTypeForPlatformEvent(initialKeyEvent);

UserGestureIndicator gestureIndicator(ProcessingUserGesture, m_frame.document(), gestureType, UserGestureIndicator::ProcessInteractionStyle::Delayed);
UserTypingGestureIndicator typingGestureIndicator(m_frame);
Expand Down Expand Up @@ -4765,7 +4799,7 @@ bool EventHandler::handleTouchEvent(const PlatformTouchEvent& event)

const Vector<PlatformTouchPoint>& points = event.touchPoints();

UserGestureIndicator gestureIndicator(ProcessingUserGesture, m_frame.document());
UserGestureIndicator gestureIndicator(ProcessingUserGesture, m_frame.document(), userGestureTypeForPlatformEvent(event));

bool freshTouchEvents = true;
bool allTouchReleased = true;
Expand Down

0 comments on commit f4cdea7

Please sign in to comment.