try evaluating the following on any page ``` navigator.share({ text: "test" }) ``` this used to work now it errors with > Unhandled Promise Rejection: NotAllowedError: The request is not allowed by the user agent or the platform in the current context, possibly because the user denied permission. T.T
<rdar://problem/84899576>
Created attachment 443053 [details] Patch v1.0
Created attachment 443057 [details] Patch v1.1 - Fix additional user gesture emulation cases
Comment on attachment 443057 [details] Patch v1.1 - Fix additional user gesture emulation cases View in context: https://bugs.webkit.org/attachment.cgi?id=443057&action=review r- as this definitely needs some tests, but otherwise the logic is sound :) One thing worth noting is that the underlying concept we're trying to emulate with this is a "activation" <https://html.spec.whatwg.org/multipage/interaction.html#activation-notification>, and in the case of `navigator.share` specifically it's a "transient activation" that's consumed on the first attempt. So we may wanna make a test that tries to call `navigator.share` twice in the same `Runtime.evaluate` and confirm that the second one fails because the first consumed the emulated "transient activation". > Source/WebCore/ChangeLog:8 > + In some code paths, including navigator.share require that we emulate user gestures for the specific Document NIT: awkward sentence, please rephrase :) > Source/WebCore/inspector/InspectorFrontendHost.cpp:113 > + document = downcast<Document>(globalObject->scriptExecutionContext()); Can we actually assume for sure that this is a `Document`? I think yes (since this is about contextmenus), but just wanna make sure :) > Source/WebCore/inspector/agents/page/PageDebuggerAgent.cpp:70 > +Document* PageDebuggerAgent::documentForInjectedScript(InjectedScript injectedScript) `const InjectedScript& injectedScript` NIT: I kinda think this would be better on `PageRuntimeAgent` since "debugging" is kinda a subset of "regular evaluation" 😅 > Source/WebCore/inspector/agents/page/PageDebuggerAgent.cpp:84 > + auto injectedScript = injectedScriptManager().injectedScriptForObjectId(callFrameId); So this means that we have two calls to `injectedScriptForObjectId` in the same callstack, since `InspectorDebuggerAgent::evaluateOnCallFrame` also does it. That's not the worst thing, but it's not super ideal either. Rather than have that, perhaps we could remove the override `PageDebuggerAgent::evaluateOnCallFrame` and introduce a new `virtual UserGestureEmulationScope InspectorDebuggerAgent::emulateUserGesture(bool, const InjectedScript&) { }` that'd be overridden by `PageDebuggerAgent` and is called inside `InspectorDebuggerAgent::evaluateOnCallFrame` that'd do the `UserGestureEmulationScope` logic instead (tho this may require that we create a base `UserGestureEmulationScope` inside JSC that's overridden in WebCore)? > Source/WebCore/inspector/agents/page/PageDebuggerAgent.cpp:140 > + Document* document = nullptr; > + if (auto* domGlobalObject = JSC::jsCast<JSDOMGlobalObject*>(globalObject)) > + document = downcast<Document>(domGlobalObject->scriptExecutionContext()); Could we also create a `static Document* PageDebuggerAgent::documentForGlobalObject(JSC::JSGlobalObject*)` so that this code isn't repeated?
Created attachment 454869 [details] [fast-cq] Patch (apologies to @Patrick Angle, I totally forgot that I'd filed this bug AND that you'd already written a patch 😅 thanks for letting me take it)
Created attachment 454888 [details] [fast-cq] Patch
Comment on attachment 454888 [details] [fast-cq] Patch rs=me, Nice tests!
Committed r291517 (248623@main): <https://commits.webkit.org/248623@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 454888 [details].