1965560 - img.width and height return 0 for dimensionless SVG images that aren't being rendered

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[Daniel Holbert [:dholbert]]

Spinning this off from bug 1935269 for the img.width and height APIs. (The patch that landed in bug 1935269 changed .naturalWidth and .naturalHeight to return nonzero values for dimensionless SVG images, but didn't fix .width and .height.)

Two testcases here:

• from bug 1935269 comment 17: https://bug1935269.bmoattachments.org/attachment.cgi?id=9470135
  (says 0 x 0 in Firefox vs. 200 x 150 in Chrome.)

• from bug 1923304 comment 19: https://bug1923304.bmoattachments.org/attachment.cgi?id=9462391
  (second line says width: 0, height: 0 in Firefox Nightly vs. width: 150, height: 150 in Chrome.)

I'm testing using Firefox Nightly 140.0a1 (2025-05-09) (64-bit), which includes the fix for bug 1935269.

Comment 1

[Daniel Holbert [:dholbert]]

The img.width and .height implementations are here:

https://searchfox.org/mozilla-central/rev/c3a321dcf29b1e209c6b6b1a612b8abe1b604926/dom/html/HTMLImageElement.cpp#611-613

uint32_t HTMLImageElement::Height() { return GetWidthHeightForImage().height; }

uint32_t HTMLImageElement::Width() { return GetWidthHeightForImage().width; }

...which both call over to the GetWidthHeightForImage helper, here:
https://searchfox.org/mozilla-central/rev/c3a321dcf29b1e209c6b6b1a612b8abe1b604926/dom/base/nsImageLoadingContent.cpp#1278

...which probably wants the same sort of fixup that we did in bug 1935269.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: testcase with <input type="image">

This bug is also relevant for <input type="image"> which also derives from nsImageLoadingContent.cpp and has the same .width and .height accessors, which show the same behavior-difference that <img> does.

Here's a variant of the first testcase that I linked in comment 0, but now using <input type="image">. As in comment 0, this says 0 x 0 in Firefox vs. 200 x 150 in Chrome.

Comment 3

[Daniel Holbert [:dholbert]]

Safari results for the three testcases discussed here so far:

• from bug 1935269 comment 17: https://bug1935269.bmoattachments.org/attachment.cgi?id=9470135
  (says 0 x 0 in Firefox vs. 200 x 150 in Chrome.)

Safari says 40x30 (literally using the values in the viewBox as an intrinsic size, which they shouldn't be doing).

• from bug 1923304 comment 19: https://bug1923304.bmoattachments.org/attachment.cgi?id=9462391
  (second line says width: 0, height: 0 in Firefox Nightly vs. width: 150, height: 150 in Chrome.)

Safari says 104x104 (again, literally using the values in the viewBox as an intrinsic size, after flooring/truncating them in this case -- in this case the viewBox is "-5 -5 104.6099853515625 104.6572265625")

And then:

https://bug1965560.bmoattachments.org/attachment.cgi?id=9486706 from comment 2 here (for <input type="image">):
this says 0 x 0 in Firefox vs. 200 x 150 in Chrome.

Safari says 0 x 0 here, presumably because whatever handling they have for this case is specific to their HTMLImageElement implementation (rather than being in their shared abstraction that corresponds to our nsImageLoadingContent).

Comment 4

[Daniel Holbert [:dholbert]]

(side note, partly for my own later reference - there's some subtlety in the relevant bits of the HTML spec about the image's "preferred density-corrected dimensions" which I want to be sure not to break as part of any refactoring here. That's about the image having information about having a preferred higher-resolution than its regular size, encoded in its EXIF data -- one sample high-res image that we use in tests is here:
https://searchfox.org/mozilla-central/source/testing/web-platform/tests/density-size-correction/resources/exif-resolution-valid-hires.jpg
...and it looks like we started honoring this data in bug 1680387.)

Comment 5

[Daniel Holbert [:dholbert]]

Actually, more directly-relevant here, that's related to the high-DPI information that can be encoded in srcset="...Nx" for some numeric N as in e.g. https://developer.mozilla.org/en-US/docs/Web/HTML/Guides/Responsive_images#resolution_switching_same_size_different_resolutions )

Here's a good example (where foo.svg is a dimensionless SVG):

<!DOCTYPE html>
<img srcset="foo.svg 3x">

In Chrome and Firefox Nightly right now, this^ has a naturalWidth of 100 and a naturalHeight of 50 (which is the 300x150 default object size scaled down by 1/3 based on the 3x there).

(We probably should add a testcase to assert that behavior.)

For this bug here with .width and .height on the image... it looks like Chrome does not scale those down. So img.width is still 300 and img.height is still 150.

This might be a Chrome bug where they're failing to apply the srcset scaling to width and height. If I do the same measurements after adding an explicit width='300' height='150' inside of my SVG image, and I view it with the same html document quoted above using 3x in srcset, then Chrome reports naturalWidth and width of 100, and a naturalHeight and height of 50.

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: Bug 1965560 part 1: Move NaturalWidth() function from HTMLImageElement to its superclass nsImageLoadingContent.cpp. r?#layout

This patch doesn't change behavior; it's just moving this function from a
derived class to its superclass (because a later patch in this series will need
to call it from the superclass).

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: Bug 1965560 part 2: Give NaturalSize() a param to let callers opt-out of density-correction. r?#layout

This patch doesn't change behavior for now (because none of the existing
callsites pass the param to opt out); but the next patch will add a callsite
that does opt out of density-correction.

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: Bug 1965560 part 3: Swap the declaration order of two variables in nsImageLoadingContent::GetWidthHeightForImage(). r?#layout

No behavior-change.

This moves the 'const nsAttrValue* value' decl to its first use where it
belongs; and moves the 'CSSIntSize size' decl a bit earlier, where we'll need
it to be in the next patch.

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: Bug 1965560 part 4: Adjust img.width/img.height internals to return the naturalWidth and naturalHeight when the img is not being rendered. r?#layout

This is to implement the spec text here, where the spec defines the IDL
attributes width and height in terms of the natural width and height when the
image is not being rendered:
https://html.spec.whatwg.org/multipage/embedded-content.html#dom-img-width

This patch diverges from that^ spec text in two key ways, for interoperability,
as described in this github comment:
https://github.com/whatwg/html/issues/11287#issuecomment-2923467541

I've called these out in the code itself, and I intend to update the spec to
address this divergence in that whatwg/html github-issue.

Comment 10

[Daniel Holbert [:dholbert]]

Reviewing the testcases that I've linked here:

(In reply to Daniel Holbert [:dholbert] from comment #0)

• from bug 1935269 comment 17: https://bug1935269.bmoattachments.org/attachment.cgi?id=9470135
  (says 0 x 0 in Firefox vs. 200 x 150 in Chrome.)

This says 300 x 225 with my patch, while Chrome still says 200 x 150 (and Safari says 40 x 30).

This discrepancy is worth looking into, but I intend to take it in a followup. (At the very least, I want to be sure it's covered in my WPT -- it apparently isn't right now, or isn't covered in a way that triggers a different result in Firefox and Chrome.)

It looks like we're falling back to the default concrete object width first (300) and then setting the height to be 3/4 of that, whereas Chrome is falling back to the the concrete object height first (150) and then setting the width to be 4/3 that.

• from bug 1923304 comment 19: https://bug1923304.bmoattachments.org/attachment.cgi?id=9462391
  (second line says width: 0, height: 0 in Firefox Nightly vs. width: 150, height: 150 in Chrome.)

We match Chrome on this one. (For both of us, the first line is sometimes 0 0, but the second line is always 150 150.)

(In reply to Daniel Holbert [:dholbert] from comment #2)

https://bug1965560.bmoattachments.org/attachment.cgi?id=9486706
Here's a variant of the first testcase that I linked in comment 0, but now using <input type="image">. As in comment 0, this says 0 x 0 in Firefox vs. 200 x 150 in Chrome.

As with the first testcase in comment 0: with my patch stack, we produce 300x225 for this testcase (vs 200x150 for Chrome).

Comment 11

[Daniel Holbert [:dholbert]]

More importantly, I confirmed that bug 1923304 and bug 1951871 (the webcompat site-report bugs here) are both fixed in a build with the attached patch stack.

Comment 12

[Daniel Holbert [:dholbert]]

Try run from a bit earlier -- just unrelated/intermittent failures, it looks like:
https://treeherder.mozilla.org/jobs?repo=try&revision=d2a70e743df3aeb4fdb91e772fdef79724e1e4e3

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b5d8f251593d
https://hg.mozilla.org/integration/autoland/rev/ce8c3ed4548e
part 1: Move NaturalWidth() function from HTMLImageElement to its superclass nsImageLoadingContent.cpp. r=layout-reviewers,emilio
https://github.com/mozilla-firefox/firefox/commit/d03ff6b8d72e
https://hg.mozilla.org/integration/autoland/rev/60c2922313f2
part 2: Give NaturalSize() a param to let callers opt-out of density-correction. r=layout-reviewers,emilio
https://github.com/mozilla-firefox/firefox/commit/1b8707555d72
https://hg.mozilla.org/integration/autoland/rev/0d11f96492d3
part 3: Swap the declaration order of two variables in nsImageLoadingContent::GetWidthHeightForImage(). r=layout-reviewers,emilio
https://github.com/mozilla-firefox/firefox/commit/67c26ea3b08c
https://hg.mozilla.org/integration/autoland/rev/f460a9730b0d
part 4: Adjust img.width/img.height internals to return the naturalWidth and naturalHeight when the img is not being rendered. r=layout-reviewers,emilio

Comment 14

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ce8c3ed4548e
https://hg.mozilla.org/mozilla-central/rev/60c2922313f2
https://hg.mozilla.org/mozilla-central/rev/0d11f96492d3
https://hg.mozilla.org/mozilla-central/rev/f460a9730b0d

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Might this be worth an ESR140 backport given the number of site issues it seems to have fixed?

Comment 16

[Daniel Holbert [:dholbert]]

Yeah, let's do it. For coherence, it really makes sense for users to have this patch along with bug 1935269 (which landed in 140) together, so backporting this to 140 makes a lot of sense.

Comment 17

[Daniel Holbert [:dholbert]]

Bug 1969753 is the tipmost patch that we'll need, so I'll generate the uplift request over there. (Lando is refusing to make the uplift request for me right now, though, so it might be a little while; asking around in matrix #conduit channel...)

Comment 18

[Daniel Holbert [:dholbert]]

Attachment: Bug 1965560 part 1: Move NaturalWidth() function from HTMLImageElement to its superclass nsImageLoadingContent.cpp. r?#layout

This patch doesn't change behavior; it's just moving this function from a
derived class to its superclass (because a later patch in this series will need
to call it from the superclass).

Original Revision: https://phabricator.services.mozilla.com/D251985

Comment 19

[Daniel Holbert [:dholbert]]

Attachment: Bug 1965560 part 2: Give NaturalSize() a param to let callers opt-out of density-correction. r?#layout

This patch doesn't change behavior for now (because none of the existing
callsites pass the param to opt out); but a later patch in this series will
add a callsite that does opt out of density-correction.

Original Revision: https://phabricator.services.mozilla.com/D251986

Comment 20

[Daniel Holbert [:dholbert]]

Attachment: Bug 1965560 part 3: Swap the declaration order of two variables in nsImageLoadingContent::GetWidthHeightForImage(). r?#layout

No behavior-change.

This moves the 'const nsAttrValue* value' decl to its first use where it
belongs; and moves the 'CSSIntSize size' decl a bit earlier, where we'll need
it to be in the next patch.

Original Revision: https://phabricator.services.mozilla.com/D251987

Comment 21

[Daniel Holbert [:dholbert]]

Attachment: Bug 1965560 part 4: Adjust img.width/img.height internals to return the naturalWidth and naturalHeight when the img is not being rendered. r?#layout

This is to implement the spec text here, where the spec defines the IDL
attributes width and height in terms of the natural width and height when the
image is not being rendered:
https://html.spec.whatwg.org/multipage/embedded-content.html#dom-img-width

This patch diverges from that^ spec text in two key ways, for interoperability,
as described in this github comment:
https://github.com/whatwg/html/issues/11287#issuecomment-2923467541

I've called these out in the code itself, and I intend to update the spec to
address this divergence in that whatwg/html github-issue.

Original Revision: https://phabricator.services.mozilla.com/D251988

Comment 22

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/bb79918fd303
https://hg.mozilla.org/releases/mozilla-esr140/rev/a06f4b8aabdb
https://hg.mozilla.org/releases/mozilla-esr140/rev/365fff026b3e
https://hg.mozilla.org/releases/mozilla-esr140/rev/32ee51dcfc3e