script: Impl UserActivation interface#42060
Conversation
8d5d0bd to
5f73f5a
Compare
UserActivation interfaceUserActivation interface
5f73f5a to
a5cb44a
Compare
|
🔨 Triggering try run (#21429463551) for Linux (WPT) |
a5cb44a to
8b3680b
Compare
|
Test results for linux-wpt from try job (#21429463551): Flaky unexpected result (24)
Stable unexpected results that are known to be intermittent (23)
Stable unexpected results (12)
|
|
|
Signed-off-by: Jo Steven Novaryo <[email protected]>
Signed-off-by: Jo Steven Novaryo <[email protected]>
Signed-off-by: Jo Steven Novaryo <[email protected]>
Signed-off-by: Jo Steven Novaryo <[email protected]>
Signed-off-by: Jo Steven Novaryo <[email protected]>
140862e to
e67cbac
Compare
|
🔨 Triggering try run (#21579578666) for Linux (WPT) |
Signed-off-by: Jo Steven Novaryo <[email protected]>
e67cbac to
106cd65
Compare
|
Test results for linux-wpt from try job (#21579578666): Flaky unexpected result (31)
Stable unexpected results that are known to be intermittent (36)
|
|
✨ Try run (#21579578666) succeeded. |
|
I've asked @kkoyung to take a first look at this PR. |
components/script/dom/window.rs
Outdated
|
|
||
| /// <https://html.spec.whatwg.org/multipage/#last-activation-timestamp> | ||
| #[no_trace] | ||
| last_activation_timestamp: Cell<CrossProcessInstant>, |
There was a problem hiding this comment.
A last activation timestamp, which is either a DOMHighResTimeStamp, positive infinity (indicating that W has never been activated), or negative infinity (indicating that the activation has been consumed). Initially positive infinity.
We could define a new enum with three variants: NegativeInfinity, TimeStamp(CrossProcessInstant) and PositiveInfinity, and derive PartialOrd trait. This is less ambiguous than using u64::MAX to represent positive infinity.
There was a problem hiding this comment.
Thanks! Sure, the definition seems quite specific to UserActivation.
components/script/dom/window.rs
Outdated
| /// <https://html.spec.whatwg.org/multipage/#sticky-activation> | ||
| pub(crate) fn has_sticky_activation(&self) -> bool { | ||
| // > When the current high resolution time given W is greater than or equal to the last activation timestamp in W, W is said to have sticky activation. | ||
| CrossProcessInstant::now() > self.last_activation_timestamp.get() |
There was a problem hiding this comment.
It should be >=, though it’s unlikely to hit this case :)
components/script/dom/window.rs
Outdated
| self.navigation_start.set(CrossProcessInstant::now()); | ||
| } | ||
|
|
||
| pub(crate) fn last_activation_timestamp(&self) -> CrossProcessInstant { |
There was a problem hiding this comment.
Will the timestamp read from the outside of window.rs? If no, we directly use self.last_activation_timestamp.get() instead of making a public function.
There was a problem hiding this comment.
For now not, it does sound unnecessary. Thanks!
kkoyung
left a comment
There was a problem hiding this comment.
We can make use of derived PartialOrd and Default trait for UserActivationTimestamp so that we don't need to manually implement PartialEq<CrossProcessInstant>, PartialOrd<CrossProcessInstant> and Default trait.
components/script/dom/window.rs
Outdated
| /// <https://html.spec.whatwg.org/multipage/#sticky-activation> | ||
| pub(crate) fn has_sticky_activation(&self) -> bool { | ||
| // > When the current high resolution time given W is greater than or equal to the last activation timestamp in W, W is said to have sticky activation. | ||
| self.last_activation_timestamp.get() <= CrossProcessInstant::now() |
There was a problem hiding this comment.
| self.last_activation_timestamp.get() <= CrossProcessInstant::now() | |
| UserActivationTimestamp::TimeStamp(CrossProcessInstant::now()) >= self.last_activation_timestamp.get() |
components/script/dom/window.rs
Outdated
| self.last_activation_timestamp.get() <= current_time && | ||
| self.last_activation_timestamp.get() >= current_time - time::Duration::milliseconds(pref!(dom_transient_activation_duration_ms)) |
There was a problem hiding this comment.
| self.last_activation_timestamp.get() <= current_time && | |
| self.last_activation_timestamp.get() >= current_time - time::Duration::milliseconds(pref!(dom_transient_activation_duration_ms)) | |
| UserActivationTimestamp::TimeStamp(current_time) >= self.last_activation_timestamp.get() | |
| && UserActivationTimestamp::TimeStamp(current_time) < self.last_activation_timestamp.get() + pref!(dom_transient_activation_duration_ms) |
But we still need to implement std::ops::Add<i64> trait for UserActivationTimestamp in useractivation.rs if we write it in this way.
| #[derive(Clone, Copy, PartialEq, MallocSizeOf)] | ||
| pub(crate) enum UserActivationTimestamp { | ||
| PositiveInfinity, | ||
| TimeStamp(CrossProcessInstant), | ||
| NegativeInfinity, | ||
| } |
There was a problem hiding this comment.
| #[derive(Clone, Copy, PartialEq, MallocSizeOf)] | |
| pub(crate) enum UserActivationTimestamp { | |
| PositiveInfinity, | |
| TimeStamp(CrossProcessInstant), | |
| NegativeInfinity, | |
| } | |
| #[derive(Clone, Copy, Default, PartialEq, PartialOrd, MallocSizeOf)] | |
| pub(crate) enum UserActivationTimestamp { | |
| NegativeInfinity, | |
| TimeStamp(CrossProcessInstant), | |
| #[default] | |
| PositiveInfinity, | |
| } |
95b918f to
ad8c46f
Compare
Updated, thanks! It definitely looks better! Just realized how does a |
| dom_testing_html_input_element_select_files_enabled: false, | ||
| dom_testperf_enabled: false, | ||
| dom_testutils_enabled: false, | ||
| dom_transient_activation_duration_ms: 5000, |
There was a problem hiding this comment.
The current activation duration is based on Firefox and Chromium where it is 5 seconds, which seems to be unchanged since the implementation. Looking to discuss whether this would be fine for us.
There was a problem hiding this comment.
The spec expects this to be at most a few seconds, so setting it to 5 seconds seems fine and aligns with Firefox and Chrome.
Signed-off-by: Jo Steven Novaryo <[email protected]>
ad8c46f to
a2d3f4f
Compare
|
I have also done some WPT analysis to take a look at implementation correctness and at what's need to be completed. Detailshtml\user-activation\message-event-init.tentative.htmlhtml\user-activation\message-event-activation-api-iframe-cross-origin.sub.tentative.html html\user-activation\activation-trigger-mouse-right.html.ini html\user-activation\activation-trigger-pointerevent.html html\user-activation\navigate-to-sameorigin.html html\user-activation\navigation-state-reset-crossorigin.sub.html html/semantics/links/links-created-by-a-and-area-elements/target_blank_useractivation.html html/capability-delegation/delegate-fullscreen-request-popup-cross-origin.https.sub.tentative.html |
This PR looks good to me now. Those failing tests mostly depend on implementation of other parts. We can open an issue with your analysis after merging this PR so that we won't lose it. |
Signed-off-by: Jo Steven Novaryo <[email protected]>
User activation is a concept used to prevents annoying usage of specific API (Fullscreen API, Virtual Keyboard) to the user by states that the API needs to have in order to process. This PR implements the
UserActivationinterface and keep track of the last user interaction. EachWindowwill keep track the last activation time stamp, which will be set if there are a triggering input event firing in theWindowand propagating across the browsing contexts ancestors and descendants. These timestamp could be consumed, and the timestamp will be set to negative infinite.It is then used to gate some APIs within browser that is determined as transient activation consuming-gated APIs, which needs transient activation to be true and consumes the activation. For the purpose of testing, this PR would implement this behavior on fullscreen API which is used to test activation consuming behavior.
Spec: https://html.spec.whatwg.org/multipage/interaction.html#the-useractivation-interface
Testing: Existing WPT