1506592 - We show the broken image icon for <img> without src attributes after bug 1196668

Status: VERIFIED FIXED

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

Description

[Jason Laster [:jlast]]

On 11/09 we landed 1503871, which converted the SVG URLs to relative urls which would not violate the security issues. Shortly after, bug 1196668 landed which required alt attributes to be present.

Adding an alt attribute works in most cases, but it effects the layout of the source tree.

Comment 1

[Jason Laster [:jlast]]

Attachment: Bug 1506592 - SVGs with a mask URL are broken. r=dwalsh

Comment 2

[Jason Laster [:jlast]]

here is a GIF of the source tree layout being fixed by removing alt attributes.  -- http://g.recordit.co/jjaUgvUStx.gif

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Attachment: Testcase.

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1506592 - Make sure to only display the broken image icon if there's a request at all.

This is enough to fix the devtools regression and matches what other browsers
do in the no-attribute case.

Also, I think this change over all makes sense: it doesn't make any sense to
display the broken image icon if there's no request, and we already assume in
EnsureIntrinsicSizeAndRatio() that we don't paint the icon for those (and make
the intrinsic size 0x0).

We still show the border, which matches other UAs (note that devtools
effectively masks the border away with mask-image).

This technically also can change behavior of <object> and <input>, but I think
it's better to be consistent, since EnsureIntrinsicSizeAndRatio also doesn't
special-case <img> either.

Comment 5

[[:philipp]]

Attachment: screenshot

this issue is also apparent on the new tab page in new profiles.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/63fca07a71b8
Make sure to only display the broken image icon if there's a request at all. r=bzbarsky

Comment 8

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/63fca07a71b8

Comment 11

[Ciprian Muresan [:cmuresan], Formerly of Ecosystem QA - please needinfo @mcoman for things regarding the team]

The issue was only partially fixed on all platforms, at least on the New Tab page. The broken icon is no longer displayed, but the weird rectangle is still there.

@Emilio, should we reopen this issue or log a new one for this behavior?

Comment 12

[Ciprian Muresan [:cmuresan], Formerly of Ecosystem QA - please needinfo @mcoman for things regarding the team]

Attachment: image.png

Attached a screenshot of the behavior described in comment 11.

Comment 13

[Emilio Cobos Álvarez [:emilio]]

Yeah, the patch here doesn't remove the border, just the broken image icon.

I could make it do so if needed, but all other UAs that show a border around broken images (that's WebKit and Blink effectively) agree on showing a border here as well, as it can be seen in: https://crisal.io/tmp/img-fallback.html

Is there any chance the front end could use any other tag, like, let's say, a <span> or so?

Does anybody have any strong opinion about that? I'm fairly ambivalent about whether we should render the border or not for the cases we don't paint the icon.

The only reason for using an <img> instead of a <span> here would be accessibility I guess (after bug 1196668 the <img> node for the a11y tree would be a graphic instead of another thing). Though what we were doing before that was effectively the same accessibility-wise than using a span.

Comment 14

[Ciprian Muresan [:cmuresan], Formerly of Ecosystem QA - please needinfo @mcoman for things regarding the team]

@andreio, could you please comment on the workarounds offered in comment 13?

Comment 15

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4564

Comment 16

[Andrei Oprea [:andreio]]

It does fix it. Thanks for the ni.

Comment 17

[[github robot]]

Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/78c4062812b803cfcafc36f4ad34303c632c4235
Fix Bug 1506592 - Broken image icon for empty sections (#4564)

Comment 18

[Ed Lee :Mardak]

Activity stream export bug 1508407 with span workaround from comment 13
https://hg.mozilla.org/mozilla-central/rev/945c7e99ee1f#l25.1147

Comment 19

[Ciprian Muresan [:cmuresan], Formerly of Ecosystem QA - please needinfo @mcoman for things regarding the team]

I have verified that the issue is no longer reproducible on the latest Nightly 65.0a1 (Build ID 20181120100045) on Windows 10 x64, Mac 10.13.3, and Arch Linux 4.16 x64.