Closed Bug 1506592 Opened Last year Closed 11 months ago

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

Categories

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

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla65
Tracking Status
firefox65 --- verified

People

(Reporter: jlast, Assigned: emilio)

References

(Blocks 1 open bug)

Details

Attachments

(5 files, 1 obsolete file)

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.
here is a GIF of the source tree layout being fixed by removing alt attributes.  -- http://g.recordit.co/jjaUgvUStx.gif
Attached file Testcase.
Assignee: nobody → emilio
Blocks: 1196668
Summary: SVGs with a mask URL are broken → We show the broken image icon for <img> without src attributes after bug 1196668
Component: Debugger → Layout: Images, Video, and HTML Frames
Product: DevTools → Core
Attachment #9024452 - Attachment mime type: text/plain → text/html
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.
Attachment #9024444 - Attachment is obsolete: true
Attached image screenshot
this issue is also apparent on the new tab page in new profiles.
Blocks: 1506804
Pushed by emilio@crisal.io:
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
Duplicate of this bug: 1506802
https://hg.mozilla.org/mozilla-central/rev/63fca07a71b8
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Duplicate of this bug: 1506918
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?
Flags: needinfo?(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.
Flags: needinfo?(emilio)
@andreio, could you please comment on the workarounds offered in comment 13?
Flags: needinfo?(andrei.br92)
It does fix it. Thanks for the ni.
Flags: needinfo?(andrei.br92)
Blocks: 1508407
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.