Bug 17760

Summary: <img> treats empty src as no image at all
Product: WebKit Reporter: Yuzhu Shen <yuzhu.shen>
Component: ImagesAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: darin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
Attachments:
Description Flags
The snapshot.
none
The patch for this bug.
none
The patch for this bug.
darin: review-
A new patch. darin: review+

Description Yuzhu Shen 2008-03-10 19:41:59 PDT
Consider the following example:

<img src="" width="100" height="100" alt="" onerror="processLoadError();">

WebKit treats src="" as no image at all and doesn't try to load. While IE and Firefox will try to load the image and (in most cases) trigger the onerror handler. (IE7 and Firefox2/3 are tested.)

The reason why IE and Firefox do so is that src="" should be regarded as a relative URI and resolved using the base URI. For example, if the base URI is "http://example.org/images/", the <img> with src="" should try to load this URI. In most cases, this will trigger onerror handler. (However, if an image "example.jpg" is set as the default document for this path, the <img> will load this "example.jpg".)

A real world example is as follows:
1. Go to http://qun.51.com/category_search.php?type1_name=%C7%E9%B8%D0%C1%BD%D0%D4
2. All missing images should be replaced with a "noclublogo.gif" (as llustrated in the snapshot).
3. WebKit doesn't do so, while IE and Firefox do.
Comment 1 Yuzhu Shen 2008-03-10 19:43:28 PDT
Created attachment 19653 [details]
The snapshot.

I will upload a patch for this soon.
Comment 2 Yuzhu Shen 2008-03-10 21:00:09 PDT
Created attachment 19654 [details]
The patch for this bug.
Comment 3 Yuzhu Shen 2008-03-10 23:41:43 PDT
Created attachment 19657 [details]
The patch for this bug.
Comment 4 Darin Adler 2008-03-11 07:46:00 PDT
Comment on attachment 19657 [details]
The patch for this bug.

This patch treats src="" the same as no src attribute at all. And the test case doesn't check for that. I think this is wrong.

To distinguish the case with no attribute at all, you should check the src value for attr.isNull().
Comment 5 Yuzhu Shen 2008-03-11 19:36:22 PDT
Hi, Darin. Thanks for your reply. :)

I think it is reasonable to check whether src is null. However, I am not sure when we will get in HTMLImageLoader::updateFromElement() with src is a null string:
1) When src is not specified, we will not get into HTMLImageLoader::updateFromElement() at all.
2) And I tried in javascript "delete some_img.src" or "some_img.src = undefined". They didn't work either.
How could I write a test case to test this?
Comment 6 Darin Adler 2008-03-11 19:43:31 PDT
(In reply to comment #5)
> 2) And I tried in javascript "delete some_img.src" or "some_img.src =
> undefined". They didn't work either.

Try this:

    some_img.removeAttribute("src")

That should work.
Comment 7 Yuzhu Shen 2008-03-17 05:45:28 PDT
Created attachment 19832 [details]
A new patch.

Modified the code according to Darin's advice.
Comment 8 Darin Adler 2008-03-17 07:47:52 PDT
Comment on attachment 19832 [details]
A new patch.

I'm pretty sure that instead of this patch you could have just changed the attr.isEmpty() to attr.isNull(); there's no reason to rearrange the code like this. I don't see any reason to not run the "load manually" case with an empty URL too.

But this is OK. r=me
Comment 9 Darin Adler 2008-03-17 07:52:15 PDT
I'll take care of landing this.
Comment 10 Darin Adler 2008-03-17 09:44:39 PDT
Committed revision 31097.