285428 - Image is reloaded when move from IFRAME to parent

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Laurent Martelli]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.5) Gecko/20050105 Galeon/1.3.19 (Debian package 1.3.19-4)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.5) Gecko/20050105 Galeon/1.3.19 (Debian package 1.3.19-4)

If you move an IMG using javascript replaceChild() from an IFRAME whose
contentDocument.location was dynamically replaced, the image is reloaded.

Reproducible: Always

Steps to Reproduce:
1. Go to http://www.bearteam.org/~laurent/image_reload/
2. Click on "reload frame (manual replace)". This will reload the "time" image
in the frame.
3. Click on "replace" in the frame. 

Actual Results:  
The time IMG above the frame is replaced by the one from the frame, ad just
after that, it is refreshed (the image is reloaded).

Expected Results:  
The image should not be reloaded.

Tested on firefox 1.0.1.

Other interesting scenari: 

1. Go to http://stan/~laurent/image_reload/ and directly click on "replace" in
the frame. The image is not reloaded.

2. Click on "onload replace". This is a variant of bug described: the image is
moved using the body' onload event. The image is loaded twice.

3. Click on "auto replace". This time, the image is moved by some javascript
inside the body of the page. The image is loaded in the IFRAME, but when moved,
the old image is still displayed. It you click a second time on "auto replace",
the image is loaded twice and refreshed.

Comment 1

[Boris Zbarsky [:bzbarsky]]

The patch in bug 286000 will fix it (specifically, the Equals() checks it adds
in image loading code).

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix for just this bug

This is still sorta intermingled with BindToTree in two places (img and input
element), but most of this diff is for this bug only.  Once BindToTree lands
(without this part of the changes), I'll post a diff to that tree.

Comment 3

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

Comment on attachment 179780 [details] [diff] [review]
Fix for just this bug

content/base/src/nsImageLoadingContent.h
+   * @param aForce If true, make sure to load the URI.  If false, only
+   *	     load if the URI is different from the currently loaded URI.

I'm not sure if aForce is the best name... it is not always forced... for
example, it doesn't skip security checks.
I can't think of a better name, though...

content/html/content/src/nsHTMLInputElement.cpp
     // Null value means the attr got unset; don't trigger on that
-    ImageURIChanged(*aValue);
+    ImageURIChanged(*aValue, PR_TRUE);

That comment seems wrong (even though you didn't change it)

Comment 4

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

> That comment seems wrong (even though you didn't change it)

oh, nevermind. it does seem right.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Note: this got sr as part of the patch for bug 286000.

Comment 6

[Matthias Versen [:Matti]]

I'm unsure if this is fixed or not: Please mark this bug fixed if you fixed this :-)

Comment 7

[Boris Zbarsky [:bzbarsky]]

Matti, I like to have a good idea that I'm not going to have to back a patch out
before I mark it fixed.... 

Fixed for 1.8b2.