299138 - onerror on images not triggered when 404 response is an image

Status: NEW

(Core :: Graphics: ImageLib, defect)

Description

[Christian Schmidt]

If the src of an <img> is a URL that returns 404, the onload event should fire
on the image.

This does not happen if the body of the 404 response is an image instead of an
HTML page, e.g. <http://dev.peytz.dk/~dr/tmp/404.php>.

In the Accept header Firefox indicates that it prefers image/png, so some
servers may be smart enough to send the 404 error page as a PNG.

Comment 1

[Christian Schmidt]

Attachment: Testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

I don't believe the fact that a 404 happened is exposed to libpr0n consumers...
so there's no way we can fire the onerror event here, since as far as the DOM
code knows there was no error.

I have to wonder why we're rendering a 404 as an image, though.  I don't think
we should be.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

imagelib probably needs to check nsIHttpChannel.requestSucceeded.

Comment 4

[Christian Schmidt]

> I have to wonder why we're rendering a 404 as an image, though.  I don't think
> we should be.
Why not? We do render 404 HTML pages.

IE with default settings hides the 404 pages. This is quite annoying.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> Why not? We do render 404 HTML pages.

And we fire onload for them, and treat them just like normal pages.  That's our
current behavior for images too (fire onload, treat like normal image), yet
you're having problems with it.  It's a simple consistency issue.  Either there
is an error, we should not render the data, and fire onerror, or things are OK,
we should render the data, and fire onload.  In my opinion, of course.

In any case, as I already said this is not a layout bug; the image library
doesn't give us enough information to change layout's behavior even if we wanted to.

Comment 6

[Christian Schmidt]

Hmm, good point. I guess we shouldn't display 404 images then, except perhaps
when they are opened directly, i.e. not embedded in a page. I don't know.

The reason I discovered this is that I browsed a server whose 404 response for
some reason is always an image. The server did not have a /favicon.ico, an thus
the 404 image appeared as proxy icon. In this particular case I don't think we
should use the 404 image as a favicon.

Comment 7

[Boris Zbarsky [:bzbarsky]]

So the problem is that if image 404s are to be shown imagelib can't just drop
the image itself.  Which means it would need to communicate the 404 to layout,
and all imagelib consumers would need to deal with it appropriately somehow
(cancel the request, maybe?)

Comment 8

[Hixie (not reading bugmail)]

Support resources: A favicon that is a 404, just like an <img> that is a 404, or
an <object> that is a 404, should be dropped. In the case of favicon, it should
cause us to use the default icon; in the case of img, it should cause the alt
text to be used; in the case of <object>, the replacement text..

If you go to the URI directly, then we should show the error message from the
server (whatever its MIME type is) since that's the primary resource.

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

code-wise, that translates to only doing such a check in LoadImageWithChannel,
but not in LoadImage.

Comment 10

[Boris Zbarsky [:bzbarsky]]

You mean the other way around, right?  And do you mean the imgILoader versions,
or the nsIImageLoadingContent versions?

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

ah, yes, I do.

yeah, I'm not sure which version I mean :) if this is done in
nsImageLoadingContent, xul:image will need additional changes...

Comment 12

[Bill Gianopoulos [:WG9s]]

Is there a spec someplace that says what a browser is supposed to do if the
server supplies fallback images for 403/404 errors?

I have seen webservers that are configures to return customized error pages for
403/404 errors in url paths other than /images, and for such errors in the
/images path, return a 1x1 transparent gif image.  Is the browser supposed to
ignore this image and use it's default error handling?  Is there a spec for
this?

Comment 13

[Bill Gianopoulos [:WG9s]]

Nevermind I misread the bug.  If onerror is specified this condition should trigger the onerror.

Comment 14

[Boris Zbarsky [:bzbarsky]]

There is no spec for this.  And comment 13 seems to be pretty much redundant with the existing comments in the bug, which I suggest you read...

Comment 15

[Boris Zbarsky [:bzbarsky]]

Joe, want to fix this?  ;)

Comment 16

[Simon Pieters [:zcorpan]]

Actually per HTML5 it's correct to use the 404 image.

[[
Whether the image is fetched successfully or not (e.g. whether the response code was a 2xx code or equivalent) must be ignored when determining the image's type and whether it is a valid image.

Note: This allows servers to return images with error responses, and have them displayed.
]] -- http://www.whatwg.org/specs/web-apps/current-work/#the-img-element

Same goes for <embed>, but <object> should respect 404, IIRC.

Comment 17

[Boris Zbarsky [:bzbarsky]]

HTML5 used to say something different back in 2005 when Hixie was commenting here, and I see no reason it might not change again.

Further, CSS requires different behavior (e.g. for background images).

So we need imagelib API to let consumers choose the behavior they want no matter what.

Comment 18

[Simon Pieters [:zcorpan]]

Just for the reckord...

IIRC, back in 2005 Hixie hadn't specced this yet.

IIRC, the reason HTML5 says what it does is because there's interop between IE/Win, Firefox, Opera and WebKit -- only IE5/Mac use the alt for 404s.

Comment 19

[Terry R.]

(In reply to comment #15)
> Joe, want to fix this?  ;)

Is there someone willing to work on this?

Comment 20

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

The severity field for this bug is relatively low, S3. However, the bug has 13 votes.
:aosmond, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 21

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.