layout: Add a query for getting the effective overflow of an element and use it in `script` by stevennovaryo · Pull Request #42251 · servo/servo

stevennovaryo · 2026-01-30T06:12:11Z

In layout calculation within script, Instead of relying on the computed values for overflow, query the overflow from layout for its used value to accurately reflect whether an element establish a scroll container. This query will be used for IntersectionObserver as well.

Testing: There are new IntersectionObserver WPT failures:

intersection-observer/isIntersecting-change-events.html
intersection-observer/containing-block.html

The failures are more than likely a false positive because intersection observation steps wasn't supposed to trigger a reflow, but it was before. It could be caused by the implementation of IntersectionObserver in major UAs being a little bit different from the spec in the calculation of threshold and isIntersecting .

stevennovaryo · 2026-01-30T06:19:45Z

+                if element
+                    .upcast::<Node>()
+                    .effective_overflow_without_reflow()
+                    .is_some_and(|overflow_axes| {
+                        overflow_axes.x != Overflow::Visible || overflow_axes.y != Overflow::Visible
+                    })
+                {


We are not supposed to use style() here anyway, since it would reflow the fragment tree. Unfortunately (or fortunately?) this fixes also reveals some failures, and /intersection-observer/animating.html becoming intermittent.

I take a look at it and it is possibly caused by either timing related issues or whether we matches the implementation of other user agents (and the WPTs) or not. There are some section within the spec that doesn't really matches the implementation of UAs.

Looking to check the failures more in the patch specific for ensuring the conformance.

I think we need a bit more investigation about why these tests are becoming intermittent. Causing a test to become intermittent isn't a great outcome.

Further investigation shows that by the time we are observing the element, the animation already went through 30% of it therefore the first observation passes records not intersecting.

servo/tests/wpt/tests/intersection-observer/animating.html

Lines 43 to 51 in d70ab4e

target.style.animation = "3s linear slideup";

setTimeout(() => {

reject("Did not get a not-intersecting notification within 2 seconds.");

}, 2000);

target.addEventListener("animationend", evt => {

reject("animationend event fired before not-intersecting notification.");

});

await new Promise(resolve => setTimeout(resolve, 500));

observer.observe(target);

I am suspecting the slow down is caused by running test in parallel in local machine, since running the single test results in stable passes.

Considering these changes, I have increased the duration of the animation to increase the leniency, as there are no need to measure such performance in WPT test anyway. What do you think of the changes?

stevennovaryo · 2026-01-30T06:29:47Z

+        if !self.has_css_layout_box() || !self.establish_scroll_container() || !self.has_overflow()
+        {
            return;
        }


Although the spec mentioned scrolling box for which I understand that we need to check whether the element has scrollable overflow. The very next clauses directly specify that we need to have an overflow as well.

This would prevent us from performing unnecessary operation on a element that is not supposed to be scrollable since overflow could be ignored further in the pipeline.

mrobinson

It feels like you should be able to make a test case where this matters. For instance, replaced elements and some table parts do not establish a scrolling box. Without a test, it's a bit hard to see what this is solving as it just adds failures.

mrobinson · 2026-01-30T07:07:41Z


        // Step 10
-        if !self.has_css_layout_box() || !self.has_scrolling_box() || !self.has_overflow() {
+        if !self.has_css_layout_box() || !self.establish_scroll_container() || !self.has_overflow()


This change moves the line further from the specification:

Step 10 from https://drafts.csswg.org/cssom-view/#dom-element-scroll:

If the element does not have any associated box, the element has no associated scrolling box, or the element has no overflow, return a resolved Promise and abort the remaining steps.

Thanks for the review!

I am renaming the Node.has_scrolling_box to Node.establishes_scroll_container, since the previous implementation is not following the definition (being closer to scroll container definition rather than scrolling box).

Honestly, I am unable to find the concrete definition in the spec. Being removed in w3c/csswg-drafts@f19b64a (being removed maybe by accident?).

I guess we could follow this definition and define scrolling box for Node and establishes scroll container maybe using the one within Element separately.

What do you think?

Ah, I wasn't aware of the recent deletion so I'll need to take a closer look. Thanks for the link.

mrobinson · 2026-01-30T07:09:54Z

-        // TODO: Is this the right thing to do? Maybe `Document` should be ignored and viewport
-        // should be represented by the root of the DOM flat tree.
-        if self.is::<Document>() {
-            return true;
-        }
-        let Some(element) = self.downcast::<Element>() else {
-            // Shadow roots and other nodes are not scrolling boxes.


Perhaps it makes sense to preserve this method since it is an implementation of the specification, but properly implement this last step?

I was deleting this before because in terms of logic it is almost identical to the former Node.has_scrolling_box and it is not being used anywhere.

stevennovaryo · 2026-01-30T08:46:20Z

It feels like you should be able to make a test case where this matters. For instance, replaced elements and some table parts do not establish a scrolling box. Without a test, it's a bit hard to see what this is solving as it just adds failures.

I guess IntersectionObserver test is more than possible. AFAIU for scrolling, the changes shouldn't add much about the correctness, but it prevent us from doing noop (trying to scroll such replaced or table element with ignored overflow) since the scroll tree already prevent us from scrolling a scroll node without a correct effective overflow.

servo-wpt-sync · 2026-02-02T07:57:52Z

🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#57481) with upstreamable changes.

servo-wpt-sync · 2026-02-02T09:38:47Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#57481).

servo-wpt-sync · 2026-02-02T10:01:18Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#57481).

stevennovaryo · 2026-02-02T10:01:51Z

Sorry @mrobinson, I have added a WPT of IntersectionObserver to test this particular case.

What do you think of the current state? Since scrolling box definition is missing, and the condition checked in scroll doesn't change whether we uses the deleted definition of scrolling box or just scroll container (because we checked whether they have overflow afterwards), we could possibly leave it as is, leave some TODOs and put up an issue about this in cssom.

servo-wpt-sync · 2026-02-02T15:45:58Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#57481).

stevennovaryo · 2026-02-02T15:49:17Z

I guess we could follow this definition and define scrolling box for Node and establishes scroll container maybe using the one within Element separately.

Added an implementation based on this. Please let me know if this align and appropriate. Thanks!

servo-wpt-sync · 2026-02-25T09:03:45Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#57481).

servo-wpt-sync · 2026-02-25T09:57:57Z

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#57481) title and body.

mrobinson · 2026-02-25T10:00:49Z

+        // > Elements and viewports have an associated scrolling box if the element or viewport has a scrolling mechanism,
+        // > or it overflows its content area and the used value of the overflow-x or overflow-y property is not
+        // > overflow/hidden or overflow/clip. [CSS3-BOX]
+        // TODO: handle scrolling mechanism.


Please expand this comment. "handle scrolling mechanism" is very vague. I'm not sure what it means.

I believe it is something that is already very vague from the spec, for now I am mentioning that it is something that depends on the spec. But we could add some possible definition like whether scrolling mechanism means that the element have a scrollbar. What do you think?

servo-wpt-sync · 2026-02-25T10:01:37Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#57481).

Signed-off-by: Jo Steven Novaryo <[email protected]>

servo-wpt-sync · 2026-03-05T09:46:26Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#57481).

Signed-off-by: Martin Robinson <[email protected]>

servo-wpt-sync · 2026-03-05T10:40:55Z

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#57481) title and body.

mrobinson

Please be careful when modifying functions or their visibility that they have the minimum visibility necessary. Generally this is pub(crate) if it needs to be public.

Signed-off-by: Martin Robinson <[email protected]>

servo-wpt-sync · 2026-03-05T10:44:44Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#57481).

servo-wpt-sync · 2026-03-05T10:48:29Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#57481).

stevennovaryo · 2026-03-06T07:58:43Z

Please be careful when modifying functions or their visibility that they have the minimum visibility necessary. Generally this is pub(crate) if it needs to be public.

Thanks! It does make more sense to expose this to crate.

…ection algo (#42204) Depends on #42251 for the overflow query. Use containing block queries to implement the iteration of step [3.2.7. Compute the Intersection of a Target Element and the Root](https://w3c.github.io/IntersectionObserver/#compute-the-intersection) within `IntersectionObserver` algorithm. Contrary to the algorithm in the spec that maps the coordinate space of the element for each iteration, we uses bounding client rect queries to get the appropriate clip rect. And, to handle the mapping between different coordinate spaces of iframes, we use a simple translation. Due to the platform limitation, currently it would only handle the same `ScriptThread` browsing context. Therefore innaccuracy any usage with cross origin iframes is expected. Testing: Existing and new WPTs. Part of: #35767 Co-authored-by: Josh Matthews [[email protected]]([email protected]) --------- Signed-off-by: Josh Matthews <[email protected]> Signed-off-by: Jo Steven Novaryo <[email protected]> Co-authored-by: Josh Matthews <[email protected]>

…and use it in `script` (servo#42251) In layout calculation within script, Instead of relying on the computed values for overflow, query the overflow from layout for its used value to accurately reflect whether an element establish a scroll container. This query will be used for `IntersectionObserver` as well. Testing: There are new `IntersectionObserver` WPT failures: - `intersection-observer/isIntersecting-change-events.html` - `intersection-observer/containing-block.html` The failures are more than likely a false positive because intersection observation steps wasn't supposed to trigger a reflow, but it was before. It could be caused by the implementation of `IntersectionObserver` in major UAs being a little bit different from the spec in the calculation of `threshold` and `isIntersecting` . --------- Signed-off-by: Jo Steven Novaryo <[email protected]> Signed-off-by: Martin Robinson <[email protected]> Co-authored-by: Martin Robinson <[email protected]> (cherry picked from commit a672397)

stevennovaryo requested review from Loirooriol, gterzian and mrobinson as code owners January 30, 2026 06:12

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

stevennovaryo commented Jan 30, 2026

View reviewed changes

stevennovaryo requested a review from jdm January 30, 2026 06:44

mrobinson requested changes Jan 30, 2026

View reviewed changes

servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jan 30, 2026

servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Feb 2, 2026

servo-wpt-sync mentioned this pull request Feb 2, 2026

layout: Add a query for getting the effective overflow of an element and use it in script web-platform-tests/wpt#57481

Merged

stevennovaryo force-pushed the effective-overflow-query branch from 14ac256 to d336f6d Compare February 2, 2026 09:35

stevennovaryo force-pushed the effective-overflow-query branch from d336f6d to 726abe6 Compare February 2, 2026 09:58

stevennovaryo requested a review from mrobinson February 2, 2026 15:49

stevennovaryo mentioned this pull request Feb 6, 2026

script: Partially implement IntersectionObserver compute the intersection algo #42204

Merged

stevennovaryo force-pushed the effective-overflow-query branch from ba7c34c to b187ff1 Compare February 25, 2026 08:51

stevennovaryo force-pushed the effective-overflow-query branch from b187ff1 to def0706 Compare February 25, 2026 09:57

mrobinson reviewed Feb 25, 2026

View reviewed changes

stevennovaryo added 3 commits March 5, 2026 17:39

Implement scrolling box

c0401fd

Signed-off-by: Jo Steven Novaryo <[email protected]>

Apply suggestion

75eec2c

Signed-off-by: Jo Steven Novaryo <[email protected]>

Apply suggestion about has_scrolling_box

58c7999

Signed-off-by: Jo Steven Novaryo <[email protected]>

stevennovaryo force-pushed the effective-overflow-query branch from ec2272a to 58c7999 Compare March 5, 2026 09:42

mrobinson approved these changes Mar 5, 2026

View reviewed changes

Comment thread components/script/dom/element.rs Outdated

Comment thread components/script/dom/element.rs Outdated

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

mrobinson changed the title ~~script: Add query for effective overflow~~ layout: Add a query for getting the effective overflow of an element and use it in script Mar 5, 2026

Apply suggestions from code review

288ef7e

Signed-off-by: Martin Robinson <[email protected]>

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

mrobinson reviewed Mar 5, 2026

View reviewed changes

Comment thread components/layout/query.rs Outdated

Apply suggestion from @mrobinson

d1905f9

Signed-off-by: Martin Robinson <[email protected]>

mrobinson enabled auto-merge March 5, 2026 10:42

mrobinson added this pull request to the merge queue Mar 5, 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 5, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 5, 2026

servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Mar 5, 2026

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

servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Mar 5, 2026

Merged via the queue into servo:main with commit a672397 Mar 5, 2026
36 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 5, 2026

	target.style.animation = "3s linear slideup";
	setTimeout(() => {
	reject("Did not get a not-intersecting notification within 2 seconds.");
	}, 2000);
	target.addEventListener("animationend", evt => {
	reject("animationend event fired before not-intersecting notification.");
	});
	await new Promise(resolve => setTimeout(resolve, 500));
	observer.observe(target);

Uh oh!

Conversation

stevennovaryo commented Jan 30, 2026 • edited by mrobinson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stevennovaryo Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

mrobinson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevennovaryo Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

stevennovaryo commented Jan 30, 2026

Uh oh!

servo-wpt-sync commented Feb 2, 2026

Uh oh!

servo-wpt-sync commented Feb 2, 2026

Uh oh!

servo-wpt-sync commented Feb 2, 2026

Uh oh!

stevennovaryo commented Feb 2, 2026

Uh oh!

servo-wpt-sync commented Feb 2, 2026

Uh oh!

stevennovaryo commented Feb 2, 2026

Uh oh!

servo-wpt-sync commented Feb 25, 2026

Uh oh!

servo-wpt-sync commented Feb 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

servo-wpt-sync commented Feb 25, 2026

Uh oh!

servo-wpt-sync commented Mar 5, 2026

Uh oh!

Uh oh!

Uh oh!

servo-wpt-sync commented Mar 5, 2026

Uh oh!

mrobinson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

servo-wpt-sync commented Mar 5, 2026

Uh oh!

servo-wpt-sync commented Mar 5, 2026

Uh oh!

Uh oh!

Uh oh!

stevennovaryo commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

stevennovaryo commented Jan 30, 2026 •

edited by mrobinson

Loading

stevennovaryo Jan 30, 2026 •

edited

Loading

stevennovaryo Jan 30, 2026 •

edited

Loading