1912115 - img with alt uses alt text client rect instead of width/height attributes in first event loop tick after insertion (131 regression)

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Dan Bjorge]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/127.0.0.0 Safari/537.36

Steps to reproduce:

See the attached repro.js (also at https://gist.github.com/dbjorge/9e9038373d686c937210d60209997722 ) for a self-contained minimal repro.

The minimal repro is a script on a test page which:

1. Inserts a new img element into the DOM which has:

• explicit width/height attributes
• non-empty alt text
• a new/unique src attribute (or disable caching in the devtools network tab before testing - just serving the src with cache-control: no-cache isn't enough)

2. In the same event loop tick, reads the new element's computed dimensions (eg, via newElement.clientHeight or getComputedStyle(newElement).height)

My motivating (non-minimal) repro is that this breaks some types of test scenarios that use the axe-core accessibility testing library, which I help maintain. It is common for users to write tests which set up the DOM in a testable state and then immediately run an accessibility scan with axe-core, which involves querying element layout properties. This test scenario usually involves testing within a "clean" browser environment, which meets the "non-cached src" requirement. https://app.circleci.com/pipelines/github/dequelabs/axe-core/6554/workflows/60b453e4-c523-4ef6-a3f1-f0170ad6715a/jobs/71764 is an example of one of our own regression tests that began failing as of Firefox 131.

Actual results:

In Firefox 131, if the script that adds the img element queries the computed dimensions within the same event loop tick where it added the element, it instead sees computed dimensions based on the shrinkwrapped dimensions of the alt text.

If the script delays for one event loop tick in between adding the img element and querying its dimensions, it sees the expected dimensions based on the width/height attributes (as expected).

Expected results:

The computed dimensions should be based on the width/height attributes, even in the same event loop tick that adds the img element. This is the observed behavior in Firefox 129, Firefox 130, and Chrome 127.0.6533.100.

Comment 1

[Dan Bjorge]

Attachment: self-contained minimal repro (node program serving a test page)

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Layout' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Jonathan Kew [:jfkthame]]

Thanks for the report & testcase!
Running mozregression points to bug 1076583 as the cause of this change.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1076583

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Attachment: More reduced test-case.

Here's a test-case that doesn't need node or anything.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

This was a pre-existing issue with srcset fwiw. The reason is that the image is still considered "broken" until the image load microtask runs.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1912115 - HTMLImageElement shouldn't be considered broken once a load task is ongoing. r=smaug

Clean-up some legacy nsImageLoadingContent state tracking while at it:

• mLoading was not used to set ElementState anymore, and was only
  checked on the "loading" attribute handling, in what seems to be a
  poor man's way of preventing lazy loading to start once an image load
  is ongoing? However that should already be covered by the image load
  not being sync and checking mLazyLoad itself.

• The depth tracking was unneeded, image notifications have been all
  async for ages.

Move some bools into nsImageLoadingContent to pack it better.

Comment 8

[Emilio Cobos Álvarez [:emilio]]

This is probably important to fix before shipping.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cc62b982b213
HTMLImageElement shouldn't be considered broken once a load task is ongoing. r=smaug

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/47562 for changes under testing/web-platform/tests

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c4b22299c9d9
Fix conflict with bug 1789578.

Comment 12

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/cc62b982b213
https://hg.mozilla.org/mozilla-central/rev/c4b22299c9d9

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 15

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Reproducible on a 2024-08-07 Nightly build on Windows 10 using the testcase from Comment 5.
Verified as fixed on Firefox 131.0b5 and Firefox Nightly 132.0a1 on Windows 10, Ubuntu 22, macOS 14.