[webkit-reviews] review denied: [Bug 42840] image element with src attribute can't be replaced by content: url() style : [Attachment 62762] Revised patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Leo Yang
<leo.yang at torchmobile.com.cn>'s request for review:
Bug 42840: image element with src attribute can't be replaced by content: url()
style
https://bugs.webkit.org/show_bug.cgi?id=42840

Attachment 62762: Revised patch
https://bugs.webkit.org/attachment.cgi?id=62762&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
This is a good catch. That code in ImageLoader is indeed wrong.

But this patch will no longer apply, due to changes to ImageLoader.cpp that
happened since it was posted.

> +	   // If RenderImage is generated content we don't update the renderer
either.

The above is not a helpful comment. It says what the code does, but not why.

As I understand the issue here, we want to change the image if this is a
renderer we created, but not if it's a renderer that was created by other code.


The fix should probably go in ImageLoader::renderImageResource, which can
return 0 if the renderer is not for the loaded image, but rather some other
renderer.

Special casing the generated content version of RenderImage seems a little bit
roundabout. Unfortunately, there is no longer a RenderImageGeneratedContent
class to add a virtual function to, so we need a new approach to figuring out
if this is an image renderer that the image loader should be overriding, or one
it should not be. Somehow we have to distinguish the renderer created in
RenderObject::createObject for generated content.