-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
navigator.userActivation API to query user activation state #4841
navigator.userActivation API to query user activation state #4841
Conversation
EWS run on previous version of this PR (hash 277db8a) |
EWS run on previous version of this PR (hash 3d1b658) |
EWS run on previous version of this PR (hash 443b261) |
EWS run on current version of this PR (hash 747db52) |
EWS run on previous version of this PR (hash 747db52) |
EWS run on previous version of this PR (hash 2daccbc) |
EWS run on previous version of this PR (hash d692d02) |
EWS run on current version of this PR (hash e5ee638) |
EWS run on previous version of this PR (hash e5ee638) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Can you add some comments in the commit message?
For instance, the link to the spec and what is being implemented.
It does not seem the whole spec is implemented (integration with MessageEvent, maybe worker support).
static Ref<UserActivation> create(Navigator&); | ||
~UserActivation(); | ||
|
||
Navigator* navigator(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given it seems from the tests MessageEvent might be able to own a UserActivation, is there a chance it gets exposed to workers as well?
I am not exactly sure how hasBeenActive/isActive would be computed in that case.
Maybe UserActivation would use its Navigator reference only in case it is called from a Document context?
In any case, I would tend to make navigator() private so that it is only used internally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want to support the MessageEvent
aspect (at least, not yet). It's not part of the HTML spec also and the PR this change is based on.
Sorry, I should have made that clear in the (non-existent) commit message.
Source/WebCore/html/UserActivation.h
Outdated
bool isActive(); | ||
|
||
private: | ||
UserActivation(Navigator&); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
explicit
Source/WebCore/page/DOMWindow.h
Outdated
@@ -185,6 +185,7 @@ class DOMWindow final | |||
MonotonicTime lastActivationTimestamp() const { return m_lastActivationTimestamp; } | |||
void notifyActivated(MonotonicTime); | |||
WEBCORE_EXPORT bool hasTransientActivation() const; | |||
WEBCORE_EXPORT bool hasStickyActivation() const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure WEBCORE_EXPORT is required at the moment.
LayoutTests/platform/mac-wk2/fast/dom/navigator-detached-no-crash-expected.txt
Show resolved
Hide resolved
EWS run on previous version of this PR (hash f7da745) |
EWS run on previous version of this PR (hash a5e2bbc)
|
EWS run on previous version of this PR (hash 46be888)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the WPT test changes mix WebKit specific changes and resync of WPT tests.
Can you clarify in the commit message what is what.
If there are WebKit specific changes, can you create a WPT PR?
return m_userActivation; | ||
} | ||
|
||
NavigatorUserActivation* NavigatorUserActivation::from(Navigator& navigator) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it be a NavigatorUserActivation&?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went with Ref<> instead...
return NavigatorUserActivation::from(navigator)->userActivation(); | ||
} | ||
|
||
RefPtr<UserActivation> NavigatorUserActivation::userActivation() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it be a Ref or a UserActivation&?
|
||
RefPtr<UserActivation> NavigatorUserActivation::userActivation(Navigator& navigator) | ||
{ | ||
return NavigatorUserActivation::from(navigator)->userActivation(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NavigatorUserActivation::from(navigator) cannot be null here probably
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would imagine not, as NavigatorUserActivation::from always assures creation.
Will do. However, the PR below re-syncs everything.
|
EWS run on previous version of this PR (hash 9fbbfb0)
|
EWS run on previous version of this PR (hash 54fe377)
|
EWS run on previous version of this PR (hash 8dbda63) |
EWS run on previous version of this PR (hash d56e52d)
|
EWS run on current version of this PR (hash a63472b)
|
https://bugs.webkit.org/show_bug.cgi?id=245240 Reviewed by Youenn Fablet and Geoffrey Garen. Implementation of the UserActivation interface: https://html.spec.whatwg.org/#the-useractivation-interface Which was added to HTML via: whatwg/html#8254 The API provides web content a view into DOMWindow::hasTransientActivation() and DOMWindow::hasStickyActivation(). This change excludes anything related "MessageEvent", which has not yet been agreed to by the WHATWG. The tests have been modified to work on WebKit infrastructure without changing the intent of the original tests. In particular: * `domains[www1]` and so on, so those are changed to `hosts[alt][]` * Domains are differentiated by using different ports. * Some of the tests are missing `await`. * Some tests would run before the body was ready. A PR has been sent to WPT to address these issues: web-platform-tests/wpt#36881 The tests are update to 1b73dcd. Some test coming form WTP are non-standard. In particular, tests that rely on `window.open()` and `.requestFullscreen()` are not spec'ed to consume user activation. This is a known issue and will be addressed as a followup in the specs and potentially as changes in WebKit. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/activation-trigger-keyboard-enter.html: * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/activation-trigger-keyboard-escape.html: * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/activation-trigger-mouse-left.html: * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/activation-trigger-mouse-right.html: * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/activation-trigger-pointerevent.html: * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/chained-setTimeout-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/chained-setTimeout.html: Renamed from LayoutTests/imported/w3c/web-platform-tests/html/user-activation/chained-setTimeout.tentative.html. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/chained-setTimeout.tentative-expected.txt: Removed. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/consumption-crossorigin.sub-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/consumption-crossorigin.sub.html: Renamed from LayoutTests/imported/w3c/web-platform-tests/html/user-activation/consumption-crossorigin.sub.tentative.html. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/consumption-crossorigin.sub.tentative-expected.txt: Removed. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/consumption-sameorigin-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/consumption-sameorigin.html: Renamed from LayoutTests/imported/w3c/web-platform-tests/html/user-activation/consumption-sameorigin.tentative.html. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/consumption-sameorigin.tentative-expected.txt: Removed. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/detached-iframe-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/detached-iframe.html: Added. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/message-event-activation-api-iframe-cross-origin.sub.tentative-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/message-event-activation-api-iframe-cross-origin.sub.tentative.html: * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/message-event-init.tentative-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/message-event-init.tentative.html: * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/navigation-state-reset-crossorigin.sub-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/navigation-state-reset-crossorigin.sub.html: Renamed from LayoutTests/imported/w3c/web-platform-tests/html/user-activation/navigation-state-reset-crossorigin.sub.tentative.html. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/navigation-state-reset-crossorigin.sub.tentative-expected.txt: Removed. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/navigation-state-reset-sameorigin-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/navigation-state-reset-sameorigin.html: Renamed from LayoutTests/imported/w3c/web-platform-tests/html/user-activation/navigation-state-reset-sameorigin.tentative.html. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/navigation-state-reset-sameorigin.tentative-expected.txt: Removed. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/no-activation-thru-escape-key-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/no-activation-thru-escape-key.html: * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/propagation-crossorigin.sub-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/propagation-crossorigin.sub.html: Renamed from LayoutTests/imported/w3c/web-platform-tests/html/user-activation/propagation-crossorigin.sub.tentative.html. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/propagation-crossorigin.sub.tentative-expected.txt: Removed. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/propagation-sameorigin-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/propagation-sameorigin.html: Renamed from LayoutTests/imported/w3c/web-platform-tests/html/user-activation/propagation-sameorigin.tentative.html. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/propagation-sameorigin.tentative-expected.txt: Removed. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/user-activation-interface-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/user-activation/user-activation-interface.html: Added. * LayoutTests/platform/gtk/fast/dom/navigator-detached-no-crash-expected.txt: * LayoutTests/platform/ios-wk2/TestExpectations: * LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/html/user-activation/chained-setTimeout.tentative-expected.txt: Removed. * LayoutTests/platform/mac-wk1/fast/dom/navigator-detached-no-crash-expected.txt: * LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/user-activation/no-activation-thru-escape-key-expected.txt: * LayoutTests/platform/mac-wk2/fast/dom/navigator-detached-no-crash-expected.txt: * Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml: * Source/WebCore/CMakeLists.txt: * Source/WebCore/DerivedSources-input.xcfilelist: * Source/WebCore/DerivedSources-output.xcfilelist: * Source/WebCore/DerivedSources.make: * Source/WebCore/Sources.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/bindings/js/WebCoreBuiltinNames.h: * Source/WebCore/html/NavigatorUserActivation.cpp: Added. (WebCore::NavigatorUserActivation::NavigatorUserActivation): (WebCore::NavigatorUserActivation::userActivation): (WebCore::NavigatorUserActivation::from): (WebCore::NavigatorUserActivation::supplementName): * Source/WebCore/html/NavigatorUserActivation.h: Added. * Source/WebCore/html/URLSearchParams.h: * Source/WebCore/html/UserActivation.cpp: Added. (WebCore::UserActivation::create): (WebCore::UserActivation::UserActivation): (WebCore::UserActivation::navigator): (WebCore::UserActivation::window const): (WebCore::UserActivation::hasBeenActive const): (WebCore::UserActivation::isActive const): * Source/WebCore/html/UserActivation.h: Added. * Source/WebCore/html/UserActivation.idl: Added. * Source/WebCore/page/DOMWindow.cpp: (WebCore::DOMWindow::hasStickyActivation const): * Source/WebCore/page/DOMWindow.h: * Source/WebCore/page/Navigator+UserActivation.idl: Added. Canonical link: https://commits.webkit.org/256572@main
Committed 256572@main (2df18f6): https://commits.webkit.org/256572@main Reviewed commits have been landed. Closing PR #4841 and removing active labels. |
2df18f6
a63472b