Skip to content

script: Fully initialize ResizeObserverEntry fields#40036

Merged
jdm merged 3 commits intoservo:mainfrom
simonwuelker:resizeobserver-init-border-box
Oct 21, 2025
Merged

script: Fully initialize ResizeObserverEntry fields#40036
jdm merged 3 commits intoservo:mainfrom
simonwuelker:resizeobserver-init-border-box

Conversation

@simonwuelker
Copy link
Copy Markdown
Contributor

@simonwuelker simonwuelker commented Oct 20, 2025

A ResizeObserverEntry stores of three different lists of size values:

  • border box sizes (border-box)
  • content box sizes (content-box)
  • content box sizes, in integral device pixels (device-pixel-content-box)

Currently, servo only stores content box sizes and leaves the other two empty:

let entry = ResizeObserverEntry::new(
&window,
target,
&content_rect,
&[],
&[&*observer_size],
&[],
can_gc,
);

It's worth noting that the device-pixel-content-box observation type is not supported yet, so the only size reported will be a zero-sized rectangle.

// TODO(#31182): add support for device pixel size calculations.
_ => Rect::zero(),

Testing: New web platform tests start to pass
Fixes #38811
Part of #39790

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 20, 2025
Comment on lines -291 to -292
/// Returning an optional calculated size, instead of a boolean,
/// to avoid recalculating the size in the subsequent broadcast.
Copy link
Copy Markdown
Contributor Author

@simonwuelker simonwuelker Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed this optimization. Keeping it around would make the code a bit messy because when we initialize the ResizeObserverEntry fields then we have one of the three size values (depending on the observation type) and need to calculate the other two. Now that we have incremental layout, I think the cost of querying an element's area (without reflow) is very low.

Also note that we can't get around querying layout when populating the ResizeObserverEntry either way.

I don't mind undoing this change if someone objects.

@simonwuelker simonwuelker force-pushed the resizeobserver-init-border-box branch from b7a4e0e to 38e423b Compare October 20, 2025 20:00
Copy link
Copy Markdown
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This is much clearer.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Oct 20, 2025
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 20, 2025
@jdm jdm enabled auto-merge October 20, 2025 23:22
@jdm jdm added this pull request to the merge queue Oct 21, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 21, 2025
Merged via the queue into servo:main with commit 99ca170 Oct 21, 2025
33 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ResizeObserver does not properly populate ResizeObserverEntry

3 participants