script: Fire scroll event on visual viewport scroll by stevennovaryo · Pull Request #42771 · servo/servo

stevennovaryo · 2026-02-23T07:39:41Z

On visual viewport scroll, fire the scroll event to the VisualViewport instances. Incidentaly, fix a bug where the pinch zoom update should be propagated to the script.

Testing: New unit test and manual WPT.

mrobinson · 2026-02-23T08:39:58Z

+    /// Whenever a visual viewport gets scrolled (whether in response to user interaction or by an
+    /// API), the user agent must run these steps:
+    /// <https://drafts.csswg.org/cssom-view/#scrolling-events>
+    // TODO: the scroll event handling should be updated and this function should follow as well


Suggested change

/// Whenever a visual viewport gets scrolled (whether in response to user interaction or by an

/// API), the user agent must run these steps:

/// <https://drafts.csswg.org/cssom-view/#scrolling-events>

// TODO: the scroll event handling should be updated and this function should follow as well

/// From <https://drafts.csswg.org/cssom-view/#scrolling-events>:

/// > Whenever a visual viewport gets scrolled (whether in response to user interaction or by an

/// > API), the user agent must run these steps:

// TODO: the scroll event handling should be updated and this function should follow as well

What do you mean by the last TODO? I'm not sure I completely follow it. Updated in what way?

It is to note that the Document::pending_scroll_event_targets is changed and thus the algorithm need some modification too. Should have move the TODO to the Document::pending_scroll_event_targets instead. It is not necessary to change it now since scrollend is not implemented i guess.

But changed in what way? And what modifications?

But changed in what way? And what modifications?

Initially, the document have two pending list of event targets.

From: https://www.w3.org/TR/cssom-view-1/#scrolling-events, which is not updated yet.

Each Document has an associated list of pending scroll event targets, initially empty.
Each Document has an associated list of pending scrollend event targets, initially empty.

But now it is merged, I remember it is because essentially all browser are doing the same.

From: https://drafts.csswg.org/cssom-view/#scrolling-events

Each Document has an associated list of pending scroll events, which stores pairs of (EventTarget, DOMString), initially empty.

Should have added that.

Just updated the comments. Please check whether it is clear. Thanks! @mrobinson

mrobinson · 2026-02-23T08:40:48Z

                // > If target is a Document, fire an event named scroll that bubbles at target.
                target.fire_bubbling_event(Atom::from("scroll"), can_gc);
-            } else if target.downcast::<Element>().is_some() {
+            } else {


What sort of non-Elements is it important to send scroll events to?

Initially the scroll only applies to Element and Document since the VisualViewport is not implemented, but now, since it is implemented we should fire to the VisualViewport as well.

It is to bring the algorithm closer to the spec as well, since the spec doesn't limit what kind of target we are firing on to.

webbeef · 2026-02-24T03:42:32Z

@stevennovaryo is that something we could use to implement #42150 ?

stevennovaryo · 2026-02-24T04:03:49Z

@stevennovaryo is that something we could use to implement #42150 ?

It is more about JS scroll event, instead of embedder. Looking at #42150, we might be able to push it from constellation maybe with some cleanup, because scroll updates is is not unified. Sending one by depending on JS event sounds very inefficient.

Signed-off-by: Jo Steven Novaryo <steven.novaryo@gmail.com>

stevennovaryo · 2026-03-12T09:40:50Z

Rebased, can we have another look for the PR? @mrobinson Thanks!

mrobinson

Looks good with some tiny nits:

mrobinson · 2026-03-12T10:40:25Z

+    /// > Whenever a visual viewport gets scrolled (whether in response to user interaction or by an
+    /// > API), the user agent must run these steps:
+    pub(crate) fn handle_visual_viewport_scroll_event(&self, visual_viewport: &VisualViewport) {
+        // Step 3.


Please include the text of steps 1 and 2 here with a TODO comment. There are other examples of this, in this file.

Sure! It is definitely better to be comprehensive, I have added the steps 1 and 2, they are basically the variable that's been passed into the function.

mrobinson · 2026-03-12T10:42:04Z

+    let eval_visual_viewport = |attr: &str| match evaluate_javascript(
+        &servo_test,
+        webview.clone(),
+        format!("window.visualViewport.{}", attr),
+    ) {
+        Ok(JSValue::Number(num)) => Some(num),
+        _ => None,
    };


Suggested change

let eval_visual_viewport = |attr: &str| match evaluate_javascript(

&servo_test,

webview.clone(),

format!("window.visualViewport.{}", attr),

) {

Ok(JSValue::Number(num)) => Some(num),

_ => None,

};

let eval_visual_viewport = |attribute: &str| match evaluate_javascript(

&servo_test,

webview.clone(),

format!("window.visualViewport.{}", attribute)

)

Ok(JSValue::Number(number)) => Some(number),

_ => None,

};

mrobinson · 2026-03-12T10:44:09Z

+    // TODO(31665): Instead of having separate list for `scroll` and `scrollend` event, the updated spec
+    //              make it store both the event target and the event type. Need to update this along with
+    //              the implementation of `scrollend`.


This is a tiny nit, but we typically don't indent TODO comments like this. Please format them like this:

Suggested change

// TODO(31665): Instead of having separate list for `scroll` and `scrollend` event, the updated spec

// make it store both the event target and the event type. Need to update this along with

// the implementation of `scrollend`.

// TODO(#31665): Instead of having separate list for `scroll` and `scrollend` event, the updated spec

// make it store both the event target and the event type. Need to update this along with the

// implementation of `scrollend`.

Signed-off-by: Jo Steven Novaryo <steven.novaryo@gmail.com>

On visual viewport scroll, fire the scroll event to the `VisualViewport` instances. Incidentaly, fix a bug where the pinch zoom update should be propagated to the script. Testing: New unit test and manual WPT. --------- Signed-off-by: Jo Steven Novaryo <steven.novaryo@gmail.com>

mrobinson reviewed Feb 23, 2026

View reviewed changes

stevennovaryo force-pushed the visual-viewport-scroll branch 2 times, most recently from 99ca390 to 0cca5a9 Compare February 23, 2026 09:01

stevennovaryo marked this pull request as ready for review February 23, 2026 09:18

stevennovaryo requested a review from gterzian as a code owner February 23, 2026 09:18

servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 23, 2026

stevennovaryo requested a review from mrobinson February 23, 2026 09:22

stevennovaryo force-pushed the visual-viewport-scroll branch 2 times, most recently from 254e83f to c9f3256 Compare February 24, 2026 02:31

stevennovaryo force-pushed the visual-viewport-scroll branch from c9f3256 to 43e5401 Compare February 25, 2026 06:31

Fire scroll event on visual viewport scroll

d6c3caa

Signed-off-by: Jo Steven Novaryo <steven.novaryo@gmail.com>

stevennovaryo force-pushed the visual-viewport-scroll branch from 43e5401 to d6c3caa Compare March 12, 2026 09:40

mrobinson approved these changes Mar 12, 2026

View reviewed changes

servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Mar 12, 2026

Apply suggestion

63ede32

Signed-off-by: Jo Steven Novaryo <steven.novaryo@gmail.com>

servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 12, 2026

mrobinson added this pull request to the merge queue Mar 19, 2026

servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 19, 2026

Merged via the queue into servo:main with commit c4cdcb7 Mar 19, 2026
33 checks passed

servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 19, 2026

Uh oh!

Conversation

stevennovaryo commented Feb 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webbeef commented Feb 24, 2026

Uh oh!

stevennovaryo commented Feb 24, 2026

Uh oh!

stevennovaryo commented Mar 12, 2026

Uh oh!

mrobinson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants