276824 - iframes forgetting thier current location.href

Status: RESOLVED INVALID

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

Description

[Graham Houston]

this is a **** to understand basicly the iframe forgets it's location an
defaults to the SRC that it was originaly set with..

not sure if it is directly related to the DOM rather than the javascript but on
the URL provided you can see an example of the happing...

Window 2 contains an iframe with the default src set to http://www.google.com
you will see a input box to the left of the screen with http://slashdot.org
click the href link beside it to set the Window 2 iframe's location.href too
this value. now minimize the Window 2 DOM window... then maximize it again...
you will notice the iframe is back to google ... it forgets its location and
sets it back to the original src value. ???

Comment 1

[Brendan Eich [:brendan]]

This is a DOM bug, if it's a bug.  Please file appropriately in the future --
the JS Engine is only for core ECMA-262 bugs.

/be

Comment 2

[Boris Zbarsky [:bzbarsky]]

The iframe is moved around in the DOM (removed from one place and inserted in
another).  When an iframe is removed from the DOM, the document and window
inside it, with all state, including the location, is torn down.  When the
iframe is inserted back into the DOM it loads the then-current value of the
"src" attribute.

Now I'm not sure whether this is specified anywhere, but that's the behavior
Mozilla implements...  I'm pretty sure we have existing complaints about other
symptoms of it.

Comment 3

[Johnny Stenback (:jst)]

Wait, didn't we fix this quite some time ago? Iframes elements now holds on to
the docshell etc and only show it when displayed, w/o reloading it etc... I
can't get to the testcase in this bug so I didn't test, but I'm fairly sure this
was fixed. Did it regress, or was it not fully fixed?

Comment 4

[Boris Zbarsky [:bzbarsky]]

jst, it was fixed for display:none iframes.  When an iframe is removed from the
DOM altogether, however, its associated docshell and window object are
destroyed.  See nsGenericHTMLFrameElement::SetDocument when null is passed in. 
I assumed there was a good reason for this (eg docshell that's no longer really
in the docshell tree, window object that needs to be removed from the window
object tree, etc).

Comment 5

[Graham Houston]

but this is plain WRONG....

if I have a WHITE car at one side of my house and I spray it BLACK then move it
to the other side of the house I would still want my car to be BLACK not back 
to WHITE..? just because I move something does not mean it should loose its 
attached attributes.?

you should be able to use appendChild to move an already existing object about 
the DOM without it effecting the objects attributes? it would only be the case 
if I was to remvoeChild then appendChild?


this is broken.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> you should be able to use appendChild to move an already existing object about 
> the DOM without it effecting the objects attributes? it would only be the case 
> if I was to remvoeChild then appendChild?

appendChild does a removeChild as needed.  This is clearly spelled out in the spec.

No one is suggesting that this behavior is correct, by the way.  All I said is
that we have existing bugs on it and that this one is a duplicate.

Comment 7

[Graham Houston]

perhaps the spec should include a moveChild ? I think the reason this works in 
IE is because IE never destroys any object!! from what I can tell... it seems 
to reuse objects located in its garbage.. bad design indeed if the case and 
mozilla does indeed do a better job. it's just that a standard way of moving an 
object does not seem to be included in the spec or have I missed something.

cheers.

Comment 8

[Graham Houston]

hold on am I reading the wrong spec?

it says "if the node already exists it is first removed"

have the mozilla coders taken this to mean use the removeChild code that 
destroys the object.? simply put remove does not mean use the removeChild 
method its how you read the spec and what the word remove means...

I think other code needs to be in place to remove the node for the appendChild
without destroying the object.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> have the mozilla coders taken this to mean use the removeChild code that 
> destroys the object.?

removeChild does not destroy objects.  And yes, the remove in question is done
via removeChild, since it needs to behave identically to removeChild in all
respects.

Comment 10

[Graham Houston]

ok so in case the spec does not cater for moving things about the DOM without 
destroying its attributes..? not a mozilla bug at all. 

Comment 11

[Boris Zbarsky [:bzbarsky]]

I'm not sure why you keep talking about destroying attributes.  The spec says
nothing about that.

Comment 12

[Graham Houston]

yes ok the attributes are not destroyed as such they simply get rest on the 
iframe back to its default that it was created with... therefore in a sense yes 
it's not correct? just like the car example above if I move the car after I 
change its colour attribute should it reset this colour attribute... no.

above:
>> you should be able to use appendChild to move an already existing object 
about 
>> the DOM without it effecting the objects attributes? it would only be the 
case 
>> if I was to remvoeChild then appendChild?

you confirmed that the appendChild uses removeChild:

>appendChild does a removeChild as needed.  This is clearly spelled out in the 
>spec.

>No one is suggesting that this behavior is correct, by the way.  All I said is
>that we have existing bugs on it and that this one is a duplicate.


this is where I see an issue, you say mozilla does a removeChild for 
appendChild as clearly spelled out in the spec. I've taken perhaps wrongly that 
the removeChild is were the fault lies, but this is just my assumpson based on 
what you said above..

>When an iframe is removed from the
>DOM altogether, however, its associated docshell and window object are
>destroyed. 

 But no matter what, the spec does not say directly to use removeChild first if 
the node already exists it simply says "removed" now this is of course whats 
needed but does remove indicate to use removeChild.. and if so is it the 
removeChild or the appendChild that has the problem with reseting the SRC 
attribute or is it something else altogether outside of this scope.


you say it uses removeChild as indicted by the spec and when this happens
its associated docshell and window object are destroyed. This is where I say 
its attributes are destroyed agreed..

lost :(

Comment 13

[Boris Zbarsky [:bzbarsky]]

The destruction of the docshell on removal from the tree is wrong.  We need to
fix that, as jst and I have both said in this bug...

Comment 14

[Gervase Markham [:gerv]]

This is an automated message, with ID "auto-resolve01".

This bug has had no comments for a long time. Statistically, we have found that
bug reports that have not been confirmed by a second user after three months are
highly unlikely to be the source of a fix to the code.

While your input is very important to us, our resources are limited and so we
are asking for your help in focussing our efforts. If you can still reproduce
this problem in the latest version of the product (see below for how to obtain a
copy) or, for feature requests, if it's not present in the latest version and
you still believe we should implement it, please visit the URL of this bug
(given at the top of this mail) and add a comment to that effect, giving more
reproduction information if you have it.

If it is not a problem any longer, you need take no action. If this bug is not
changed in any way in the next two weeks, it will be automatically resolved.
Thank you for your help in this matter.

The latest beta releases can be obtained from:
Firefox:     http://www.mozilla.org/projects/firefox/
Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html
Seamonkey:   http://www.mozilla.org/projects/seamonkey/

Comment 15

[Gervase Markham [:gerv]]

This bug has been automatically resolved after a period of inactivity (see above
comment). If anyone thinks this is incorrect, they should feel free to reopen it.

Comment 16

[Boris Zbarsky [:bzbarsky]]

This is a real issue.  Reopening.

Comment 17

[William Bumgarner [:zsinj]]

I'm looking for possible dupes. Bug 254144 looks related.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Yeah, indeed.

Comment 19

[:Gijs (he/him)]

So, I'm not sure why this is a dep instead of a dupe, but I bow to bz's expertise in DOM. This is surely not blocker severity, though, and seeing as the decision to use dep not dupe was made, I'm removing the dupeme whiteboard status :-)

Comment 20

[Boris Zbarsky [:bzbarsky]]

I think we could fix this without fixing bug 254144, or possibly even vice versa.  Hence not a dup.  ;)

Comment 21

[Hixie (not reading bugmail)]

FWIW, HTML5 is clear that this is a bug. The nested browsing context is defined independent of whether the element is in the document or not.

   http://www.whatwg.org/specs/web-apps/current-work/#the-iframe

Comment 22

[treeform]

The source of the iframe might live deeper then the JS DOM api - take a look at at:
 
https://bugzilla.mozilla.org/show_bug.cgi?id=301325
https://bugzilla.mozilla.org/show_bug.cgi?id=363840

This bug appears to be a symptom of some sort of iframe-with-stuck-src issue. 

Comment 23

[Brett Stime]

Possibly related to bug 279048

Comment 24

[JKL]

Attachment: test case

The original test URL associated with the bug is inaccessible.  This test case illustrates the underlying problem, which is that the DocShell child array and SHEntry array get out of sync.  You only see the problem after your shell history is intialized, so reload the page at least once.

The first time about:credits is loaded, it is the first frame on the page, and so the content is loaded using aChildOffset = 0.  The aChildOffset parameter determines what content is loaded from the history.

-134490112[7fffebd12040]: nsDocShell[7fffd1d43800]: loading about:credits with flags 0x00000000
Breakpoint 1, nsDocShell::GetChildSHEntry (this=0x7fffdfc94400, aChildOffset=0, aResult=0x7fffffffcdc0)
    at /home/jkl/firefox/docshell/base/nsDocShell.cpp:3133

Then about:logo is loaded with aChildOffset = 1.

-134490112[7fffebd12040]: nsDocShell[7fffd1dd4800]: loading about:logo with flags 0x00000000
Breakpoint 1, nsDocShell::GetChildSHEntry (this=0x7fffdfc94400, aChildOffset=1, aResult=0x7fffffffcdc0)
    at /home/jkl/firefox/docshell/base/nsDocShell.cpp:3133

Then we remove the the about:credits iframe and add it back.  But now it's the second frame in the page, so it uses aChildOffset = 1.

-134490112[7fffebd12040]: nsDocShell[7fffd1d42800]: loading about:credits with flags 0x00000000
Breakpoint 1, nsDocShell::GetChildSHEntry (this=0x7fffdfc94400, aChildOffset=1, aResult=0x7fffffffb100)
    at /home/jkl/firefox/docshell/base/nsDocShell.cpp:3133

Unfortunately, the content is loaded not according to the src attribute, but the position of the frame in the page.  Since it is the second frame, it gets the content associated with the second frame.  But when the history was set up, the second frame was about:logo.  We end up with two copies of the about:logo content.

I conclude the root cause of this bug is that the implementation incorrectly assumes the DocShell child array and SHEntry array agree on what content is where.

Comment 25

[Boris Zbarsky [:bzbarsky]]

Comment 24, while a real issue that has existing bugs covering it, has nothing to do with this bug as originally reported.  This bug as originally reported doesn't involve session history at all.

Comment 26

[JKL]

Attachment: better recreation of original behavior

Perhaps comment 24 does not explain clearly enough how the test case relates to the behavior described in the original bug posting.  This test case is a more strict recreation of the original posting, which I gather from the discussion worked like this:

1.  iframe src changed by script.
2.  iframe removed and added back
3.  iframe has content associated with original URL, not updated one

This time we don't need the breakpoint, because the whole story is told in the debug logs.  First, we load about:credits, which is the original URL for the iframe.  This content gets added to the session history.

-134490112[7fffebd12040]: nsDocShell[7fffd20b7c00]: loading about:credits with flags 0x00000000
-134490112[7fffebd12040]: nsDocShell[7fffd20b7c00]::AddToSessionHistory("about:credits", [http://www.mozilla.org/credits/])

Next the script changes the frame to about:logo.  Note this content does not get added to the session history.

-134490112[7fffebd12040]: nsDocShell[7fffd20b7c00]: loading about:logo with flags 0x00000000

When we remove the frame and add it back, the content is loaded from the session history.

-134490112[7fffebd12040]: nsDocShell[7fffd1e80c00]: loading about:logo with flags 0x00000000
-134490112[7fffebd12040]: nsDocShell[7fffd1e80c00]: loading from session history

What is in the session history?  The session history contains the content that was in the page during load time, about:credits.  We are loading the right URL-- about:logo, but we get the wrong content.

Therefore this bug is due to a problem between the docshell and session history.

Comment 27

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.

Comment 28

[Anne (:annevk)]

After comment 21 HTML was updated again to make this a feature. We don't keep a nested browsing context alive when its corresponding element is removed from the document tree.