Summary: | <img> treats empty src as no image at all | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yuzhu Shen <yuzhu.shen> | ||||||||||
Component: | Images | Assignee: | Darin Adler <darin> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | darin | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | Windows XP | ||||||||||||
Attachments: |
|
Description
Yuzhu Shen
2008-03-10 19:41:59 PDT
Created attachment 19653 [details]
The snapshot.
I will upload a patch for this soon.
Created attachment 19654 [details]
The patch for this bug.
Created attachment 19657 [details]
The patch for this bug.
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().
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? (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. Created attachment 19832 [details]
A new patch.
Modified the code according to Darin's advice.
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
I'll take care of landing this. Committed revision 31097. |