[Webkit-unassigned] [Bug 26285] Removing an image's src attribute, or setting it to null, via scripting, doesn't change the image

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26285


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #221436|review?                     |review-
               Flag|                            |




--- Comment #21 from Alexey Proskuryakov <ap at webkit.org>  2014-01-17 17:14:11 PST ---
(From update of attachment 221436)
View in context: https://bugs.webkit.org/attachment.cgi?id=221436&action=review

Thank you for looking into this, this is a good bug to fix.

I think that this change makes the code substantially more fragile and confusing though. Some comments inline.

More importantly, this needs a lot more test coverage:

- what happens when initial src points to a 404 resource, and we later remove the attribute?
- what happens when removing src from an element that's currently loading (this test only runs after unload)?
- what are the DOM events being dispatched in all of these cases?

I think that adding such tests may prompt some code changes too.

> Source/WebCore/loader/ImageLoader.cpp:259
> +        } else
> +            updateRenderer();

The added code reads really strangely. It's essentially:

if (newImage == oldImage)
    updateRenderer();

How is this a good thing? What are the other cases when this function is called with newImage == oldImage, and can updating the renderer have unintended consequences for those?

And why do we get the same CachedImage in this case when removing the src? Conceptually, there should be no CachedImage, because we are not loading anything.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.