Skip to content

Ensure that img dimensions are derived synchronously - #13057

Open
TimvdLippe wants to merge 6 commits into
whatwg:mainfrom
TimvdLippe:fix-dimensions-of-images
Open

TimvdLippe wants to merge 6 commits into
whatwg:mainfrom
TimvdLippe:fix-dimensions-of-images

Conversation

@TimvdLippe

@TimvdLippe TimvdLippe commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Browsers immediately resolve dimensions for an <img> that is a child
of a <picture> with a preceding <source> sibling. This means that adding a <source> and then computing the style of the <img> should use the dimensions of the <source>.

However, the spec didn't account for this, as it first schedules a task.
That way, only after the task would be resolved, the dimensions would
be computed.

Instead, the source set should synchronously be updated. In the task, the source set selects an image source, which again updates the
source set. But moving the "select a source set" before the task could
mean that an "error" event would be fired too soon.

As such, only updating the source set but leaving the rest as-is ensures
the relevant new tests pass and no regressions are introduced.

Fixes #8207

(See WHATWG Working Mode: Changes for more details.)


/images.html ( diff )

Browsers immediately resolve dimensions for an `<img>` that is a child
of a `<picture>` with a preceding `<source>` sibling. This means that
adding a `<source>` and then computing the style of the `<img>` should
use the dimensions of the `<source>`.

However, the spec didn't account for this, as it first schedules a task. That
way, only after the task would be resolved, the dimensions would be
computed.

Instead, the sourceset should synchronously be updated. In the task the
sourceset selects an image source, which again updates the sourceset.
But moving the "select a source set" before the task could mean that
an "error" event would be fired to soon.

As such, only updating the sourceset but leaving the rest as-is ensures
the relevant new tests pass and no regressions are introduced.
@zcorpan zcorpan changed the title Ensure that img dimensions are derived synchronously Ensure that img dimensions are derived synchronously Oct 12, 2026
@zcorpan

zcorpan commented Oct 12, 2026

Copy link
Copy Markdown
Member

Thanks for the PR!

AI review

Reviewed at dca1812.

Blockers

  • source:33799 (update the source set, the "child is el" branch): when no source matches, nothing resets dimension attribute source back to the img. So doing this synchronously still doesn't make the spec match picture-aspect-ratio.html's "Make sure style gets invalidated correctly when the source gets removed". After removeChild(source), the img keeps the detached source as its dimension attribute source, which gives "auto 200 / 100" where the test expects "auto 250 / 50". Consider setting dimension attribute source to el at the start of update the source set, which is what Chromium does (source_ = nullptr in FindBestFitImageFromPictureParent, html_image_element.cc:510).

Suggestions

  • source:33237-33243: the new step comes after the "cannot support images" return (source:33156) and after the list-of-available-images abort (source:33231), so neither path updates the source set. Case: <picture><source srcset=x width=100 height=100><img src=a></picture> with a cached. Moving the img out of the picture aborts early, and the stale source keeps mapping width/height. With images disabled, source width/height never apply at all. Chromium runs SelectSourceURL (which sets source_) before ImageLoader::UpdateFromElement, so images being disabled doesn't affect it there. Consider moving the step up to just after step 1.
  • source:32980: when obtaining images on demand, update the image data doesn't run on relevant mutations, so the dimension attribute source stays stale there too. Either update the source set directly from the relevant-mutations hook, or confirm the on-demand case is intentionally left out.
  • source:33240-33242: the note links img-dimensions, which is the width/height getters algorithm. What actually changes is the dimension attribute source mapping that rendering and getComputedStyle() see. Consider linking concept-img-dimension-attribute-source instead.

Nits

  • Commit message: "fired to soon" should be "too soon", "sourceset" should be "source set".

("when obtaining images on demand" is basically images disabled, but the user can choose to load specific images from the context menu or similar.)

@TimvdLippe

Copy link
Copy Markdown
Contributor Author

Confirmed with servo/servo#48902 that indeed those fixes makes all tests in /html/rendering/replaced-elements/attributes-for-embedded-content-and-images/picture-aspect-ratio.html pass. Running the full test suite to see if it regresses in any other way, but I think we are good to go.

aniebietafia pushed a commit to aniebietafia/servo that referenced this pull request Oct 14, 2026
…ved (servo#48902)

Based on reviewer feedback in
whatwg/html#13057

Testing: new WPT passes

Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Should we select an image source synchronously when updating the image data?

2 participants