height of IMG tag without src attribute set is 0 in Chrome, vs. line-height in Firefox (standards mode <img alt="..."> behavior)

RESOLVED FIXED in Firefox 65

Status

()

defect
RESOLVED FIXED
4 years ago
16 days ago

People

(Reporter: hsteen, Assigned: emilio)

Tracking

(Blocks 1 bug)

29 Branch
mozilla65
Points:
---
Dependency tree / graph
Bug Flags:
webcompat +

Firefox Tracking Flags

(firefox65 fixed)

Details

(Whiteboard: [webcompat:p2][wptsync upstream], URL)

Attachments

(4 attachments, 2 obsolete attachments)

If there's markup like <img> or <img width="200"> and you read element.height, what should it return?

Chrome says 0, and we've found a site depending on this behaviour - see 
https://github.com/webcompat/web-bugs/issues/463#issuecomment-132560088

It seems Firefox currently gives the line-height of the context or something like that.
I think this is because we fall back to rendering like a <span>, to display whatever alt-text happens to be provided. (in this case, the empty string)

i.e. we render these URIs the same:
  data:text/html,<img style="border:1px solid black">
  data:text/html,<span style="border:1px solid black"></span>
...whereas Chrome does not (they agree with us on the <span>; they disagree on the <img>).
IMO, our behavior here makes sense according to CSS2 rules for sizing inline elements.

The CSS2 section on calculating heights distinguishes between "Inline, non-replaced elements" (e.g. a span) and "Inline, replaced elements" (e.g. a video, or an image that has image data to render):
 https://drafts.csswg.org/css2/visudet.html#inline-non-replaced
And an <img> with no image data (no src attribute) is *non-replaced*, in the sense that it's not being filled in with image data. So, it's sized like a *non-replaced* element, i.e. like a <span>.

So, I think our behavior makes sense, according to the spec.

(I'm not sure what Chrome's doing, but I suspect they're (1) considering the element to be replaced, and (2) considering the element to *have* an intrinsic height, of 0. Given those assumptions, I think their behavior makes sense.)
Summary: height of IMG tag without src attribute set should be 0 → height of IMG tag without src attribute set is 0 in Chrome, vs. line-height in Firefox
For comparison, using <img style="border:1px solid black"> in standards-mode:
 - MS Edge matches Chrome -- it renders just a small dot, and renders a 0-height wide element if I add a 'width' attr.
 - IE 11 does its own thing -- it renders a 30x30 square, and renders a taller square if I add a 'width' attr.
 - Opera 12 (Presto) does yet another different thing -- it renders an approximately-line-height thing that just says the word "Image".

If we disregard IE11 and Opera 12 as deprecated, then that leaves us as the odd one out, in terms of our rendering behavior for <img> without a src attribute, among modern browsers.  So, even though our behavior is defensible from a spec perspective (comment 3), it might be worth considering a switch.
(Note: I didn't test any webkit-based browsers; I'm assuming they behave approximately like Chrome, too.)
Gecko's behavior of treating image elements without data loaded as much more like regular inlines might not be something all browsers do -- and it might be something that now gets us into Web compatibility trouble.
Making img's always create an nsImageFrame (instead of an inline frame before we have decoded the size of the image and then an image frame) would simplify the code too.
True.  It would also make alt text not really usable unless the alt text is extremely short and fits in the image box.  :(  Note that the HTML spec does cover this situation and we follow the spec; Chrome unfortunately does not.

Now what we could do is not fall back to the inline frame case if there is no alt text provided at all.  That might be a sensible thing to do, and may be compliant with the HTML spec; I haven't checked what the exact verbiage there is recently.

Updated

8 months ago
Product: Core → Core Graveyard

Updated

8 months ago
Product: Core Graveyard → Core
Possibly related:

data:text/html,<!doctype html><img style="border:black" width=100 height=100>

Renders nothing in Nightly and a black border in Chrome. This shows up in Gmail messages with images not shown, since it takes no space the layout collapses awkwardly.
Summary: height of IMG tag without src attribute set is 0 in Chrome, vs. line-height in Firefox → height of IMG tag without src attribute set is 0 in Chrome, vs. line-height in Firefox (standards mode <img alt="..."> behavior)
I think we should consider changing the spec for the "no src, no alt text" case to not render as an inline.  The reason to render as an inline is to show the alt text sanely; if there is no alt text that's not a consideration.

Concretely, in spec terms, this would change the "If the element is an img element that represents nothing and the user agent does not expect this to change" case in <https://html.spec.whatwg.org/multipage/rendering.html#images-3>.  That would cover cases with no alt attr or alt="" and cover cases where the src is "" or not set at all or the load failed.

Does that sound plausible?
Flags: needinfo?(annevk)

Comment 11

7 months ago
Yeah, though it's not clear to me what the new expectation would be. Treat the element as a replaced element whose content is nothing?

If the element has intrinsic dimensions we're still supposed to ignore them?
Flags: needinfo?(annevk)
> If the element has intrinsic dimensions we're still supposed to ignore them?

If the element has intrinsic dimensions and no alt attribute, that falls into the second case under https://html.spec.whatwg.org/multipage/rendering.html#images-3 to start with.

If it has explicit alt="", then we would fall into the case comment 10 is talking about.  Honestly, I have no strong opinions on how it should render.  If we don't make any special provisions apart from  my proposed change in comment 10, then it will render just like it would if we added this case to the cases in item 2 (so changed "the element has no alt attribute" to "the element has no alt attribute or its value is empty" or some such).

Comment 13

7 months ago
"the element has no alt attribute or represents nothing"? And then we could remove the fourth case? In any event, changing this seems fine if other browsers already do it.
> And then we could remove the fourth case?

No, because the second case is conditioned on having intrinsic dimensions.
Duplicate of this bug: 1502074
(Assignee)

Updated

5 months ago
Duplicate of this bug: 1505403
Flags: webcompat?
Whiteboard: [webcompat]
(Assignee)

Comment 17

5 months ago
FWIW, Blink and WebKit also honor 'title' instead of alt... :(
(Assignee)

Comment 18

5 months ago
Ugh, blink's logic at least (haven't checked WebKit) depends on 'specified dimensions' even in standards mode:

https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/html/html_image_fallback_helper.cc?l=115&rcl=ff6da4e16a772f2026a129de8cba0951c2cc85f6
Attachment #9023306 - Attachment is obsolete: true
Yes, Blink generally does nothing resembling the spec here, with the result that it's common to get completely unreadable alt text in Blink.
(Assignee)

Comment 20

5 months ago
Behavior-wise this only removes the HasAttr(src) check, and adds the IsEmpty()
check to the alt attribute value, since this function is only called for <img>
and <input>.

But it also cleans up a bit.
See Also: → 851048
(Assignee)

Updated

5 months ago
See Also: → 1505657
(Assignee)

Comment 21

5 months ago
Only a few of the image wrappers compared against about:blank but I updated all
of them so people don't get confused if they try to do it.

test_invalid_img.xhtml was a crashtest.

I had to workaround bug 1505657 in another test.

Comment 22

5 months ago
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/mozilla-inbound/rev/803b224d52a0
Make images without alt generate a replaced box regardless of src. r=bz

Comment 24

5 months ago
Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e608b4d239b
Backed out changeset 803b224d52a0 for failures at dom/html/reftests/468263-2.html on a CLOSED TREE

Comment 25

5 months ago
Pushed by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff03b0b8ed81
Make images without alt generate a replaced box regardless of src. r=bz
Backed out 2 changesets (bug 1196668) for assertion count failures at child-constraints/available-size-for-percentages-vrl-htb.https.html

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/c6a0886cba2712f0e7c602734945109b51a87779

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=success%2Cpending%2Crunning%2Ctestfailed%2Cbusted%2Cexception&searchStr=linux%2Cdebug%2Cweb%2Cplatform%2Ctests%2Ctest-linux32%2Fdebug-web-platform-tests-reftests-1%2Cw%28wr1%29&revision=803b224d52a0940b4fb4b3b9cffc6a1fa6e5d4ee

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=210723007&repo=mozilla-inbound&lineNumber=18095

Log snippet: 

[task 2018-11-09T02:31:39.359Z] 02:31:39     INFO - TEST-START | /css/css-layout-api/child-constraints/available-size-for-percentages-vrl-htb.https.html
[task 2018-11-09T02:31:39.367Z] 02:31:39     INFO - PID 7047 | 1541730699361	Marionette	INFO	Testing https://web-platform.test:8443/css/css-layout-api/child-constraints/available-size-for-percentages-vrl-htb.https.html == http://web-platform.test:8000/css/css-layout-api/green-square-ref.html
[task 2018-11-09T02:31:39.383Z] 02:31:39     INFO - PID 7047 | ++DOMWINDOW == 59 (0xdca24400) [pid = 7047] [serial = 59] [outer = 0xe2afb550]
[task 2018-11-09T02:31:39.444Z] 02:31:39     INFO - PID 7047 | ++DOMWINDOW == 60 (0xdca25800) [pid = 7047] [serial = 60] [outer = 0xe2afb550]
[task 2018-11-09T02:31:39.472Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.473Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.474Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.475Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.475Z] 02:31:39     INFO - PID 7047 | nsBlockReflowContext: Block(div)(0)@dcae8798 metrics=858994972,6000!
[task 2018-11-09T02:31:39.475Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.475Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.477Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.478Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.479Z] 02:31:39     INFO - PID 7047 | nsBlockReflowContext: Block(div)(0)@dcae8798 metrics=858994972,6000!
[task 2018-11-09T02:31:39.481Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.481Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.482Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.484Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.484Z] 02:31:39     INFO - PID 7047 | nsBlockReflowContext: Block(div)(0)@dcae8798 metrics=858994972,6000!
[task 2018-11-09T02:31:39.484Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.486Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.487Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.487Z] 02:31:39     INFO - PID 7047 | [7047, Main Thread] ###!!! ASSERTION: Our containing block must not have unconstrained inline-size!: 'aCBSize.ISize(aWM) != NS_UNCONSTRAINEDSIZE', file /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 6194
[task 2018-11-09T02:31:39.488Z] 02:31:39     INFO - PID 7047 | nsBlockReflowContext: Block(div)(0)@dcae8798 metrics=858994972,6000!
[task 2018-11-09T02:31:39.504Z] 02:31:39     INFO - PID 7047 | 1541730699500	Marionette	INFO	Found 65200 pixels different, maximum difference per channel 255
[task 2018-11-09T02:31:39.521Z] 02:31:39     INFO - Got chrome assert count 16
[task 2018-11-09T02:31:39.523Z] 02:31:39     INFO - TEST-UNEXPECTED-FAIL | /css/css-layout-api/child-constraints/available-size-for-percentages-vrl-htb.https.html | assertion count 16 is more than expected 0 assertions
[task 2018-11-09T02:31:39.524Z] 02:31:39     INFO - TEST-FAIL | /css/css-layout-api/child-constraints/available-size-for-percentages-vrl-htb.https.html | took 165ms
Flags: needinfo?(emilio)
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13996 for changes under testing/web-platform/tests
Whiteboard: [webcompat] → [webcompat][wptsync upstream]
Upstream PR was closed without merging
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Upstream PR was closed without merging
(Assignee)

Updated

5 months ago
See Also: → 1506178
Flags: webcompat? → webcompat+
Whiteboard: [webcompat][wptsync upstream] → [webcompat:p2][wptsync upstream]

Comment 31

5 months ago
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8f6772dab9a
Make images without alt generate a replaced box regardless of src. r=bz
(Assignee)

Comment 32

5 months ago
I annotated them since the bug is pre-existing.
Flags: needinfo?(emilio)
(Assignee)

Updated

5 months ago
Assignee: nobody → emilio
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #19)
> Yes, Blink generally does nothing resembling the spec here, with the result
> that it's common to get completely unreadable alt text in Blink.

Did someone file a bug on Blink for the case(s) where it deviates from the spec?
And it looks like https://bugs.chromium.org/p/chromium/issues/detail?id=753868 also exists and is a bit over a year old.

Unfortunately, the people engaging from the Blink side seem pretty confused about what the spec is saying.  :(
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 37

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c8f6772dab9a
Status: NEW → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Upstream PR merged
Attachment #9023507 - Attachment is obsolete: true
Some of the devtools images began to look broken after this change landed on friday.

I think this happens because we are using img with a mask url property so that we can load an SVG and set the color with the background property.

Adding an alt attribute fixes it, but I wonder if it makes sense to require an alt attribute.
(Assignee)

Comment 40

5 months ago
Which markup do you use?

Mind opening a separate bug for that, ideally with a reduced example? (I don't even know what I'm supposed to look at in that screenshot, I guess the debugger icons? :))

I'm happy to look into it. Sorry for the breakage :(
Flags: needinfo?(jlaster)
(Assignee)

Updated

5 months ago
Depends on: 1506592
Depends on: 1507114
Flags: needinfo?(jlaster)

Updated

3 months ago
Duplicate of this bug: 1519632
You need to log in before you can comment on or make changes to this bug.