527897 - nsDocShell::LoadURI() retrieves URI from session history when it does not make sense to do so

Status: RESOLVED WORKSFORME

(Core :: DOM: Navigation, defect)

Description

[Oded Arbel]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.4) Gecko/20091027 Fedora/3.5.4-1.fc12 Firefox/3.5.4
Build Identifier: 

A recurring phenomena reported in many bugs is that when a page containing iframes is reloaded, if the iframe set up is not completely static then the URI used to load the iframe may be different then the URI provided in the source attribute.

Background: when reloading a page from history nsDocShell::LoadURI() ignores the URI provided by the parent document and tries to lookup the last URI for iframes in the session history under the assumption that if a user was browsing an iframe to a different URI, then that last URI should be loaded instead of the one provided by the document. See the code here: http://hg.mozilla.org/mozilla-central/annotate/259c8a2fbef4/docshell/base/nsDocShell.cpp#l1182


Reproducible: Always

Steps to Reproduce:
The problem is that the lookup of which iframe's session history to use is done solely based on the location of the iframe in the document current list of iframes. As a result this lookup often retrieves a wrong URI and loads the wrong document. A non-exclusive list of conditions that break this logic follows:
- The source document has changed and when reloaded points an iframe to a different URL (Bug #478273), or a script chooses at runtime which URL to load ( Bug #279048 and Bug #517381)
- iframes are generated dynamically by a script that takes time to load - when there is more then 1, a race condition happens where sometimes they are loaded in a different order and then LoadURI() chooses a URI that belongs to one iframe for another (Bug #462076).
- iframes are removed and reinserted dynamically (Bug #464064)
- iframes are added and removed with different URIs (Bug #523418)

Actual Results:  
when an iframe is loaded, it may be loaded with a content different then expected: even though looking at the DOM tree shows the correct URI - Gecko actually went and used a different URI from the session history to load the content.

Expected Results:  
nsDocShell::LoadURI() should either choose a session history to lookup that is relevant to the correct iframe, or not use session history at all.

Disabling this behavior completely is probably not desired as it will cause a regression in the handling of reloading in a multi-frame webpage - after frame navigation, a reload will reset all the frames to their initial content.

The heuristic for using URIs from session history should be fixed so that it makes sense for non-static documents, or at least more stable.

One such simple heuristic may be to use the iframe's ID attribute if it exists and lookup only session histories for iframes with the same ID attribute. This will allow application developers to choose when to take advantage of session history and when not to do so.

Comment 1

[Oded Arbel]

Added Boris Zbarsky and olli.pettay as per Boris' request.

Comment 2

[Boris Zbarsky [:bzbarsky]]

I thin we need to bite the bullet and get this sorted out.... nominating for 1.9.3.

Comment 3

[Johnny Stenback (:jst)]

Smaug, is this likely something that will be fixed by your session history work?

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Yeah, sounds like so. I'll test.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

What's the bug number for the session history work? Should we set a dependency? This isn't a beta2 blocker, moving to betaN

Comment 6

[Boris Zbarsky [:bzbarsky]]

> What's the bug number for the session history work?

Bug 462076.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Oded, could you explain what the testcase should do? It is not clear
from the testcase.
And could you perhaps try a nightly build tomorrow?

Comment 8

[Oded Arbel]

Sorry for not writing more documentation in the test case. The test case in bug #523418 tries to simulate the behavior of the application I was writing at the time by issuing a call to some random url and then followed by two or three predictable URLs.

To use the test case do as thus:
1. Go to the page http://geek.co.il/articles/mozilla-cache-error.html
2. Clear your cache
3. Start Firebug network monitor
4. Load the page again. You should see in the network monitor that the page loads a random question URL followed by "correct1" and "correct2", and finally an image named "window-onload-complete" to mark that the page's onload event has fired.
5. Click the "start broken behavior" button. the page should load another iframe named "wrong".
7. Click on the link to go to the next page. The next page tries to load a random question then "correct1" and "correct2" again, followed by "window-onload-complete".

Under the broken behavior, on the second page the either the question or correct1 would get the contents of "wrong" instead and you can see that by looking at the response in the network monitor.

In the current 3.6 nightly I can't reproduce the broken behavior, though it easily reproduces in 3.5.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #8)
> In the current 3.6 nightly I can't reproduce the broken behavior, though it
> easily reproduces in 3.5.
What about Firefox 4 nightly?

Comment 10

[Oded Arbel]

nah - can't reproduce either.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Oded, any chance to get a testcase which doesn't require Firebug?

Comment 12

[Oded Arbel]

I don't see how I could do this. The issue for me happens when you load iframes and then remove them. As I have to remove them for this to (not)work, you won't be able to just see that the content is wrong.

Comment 13

[Boris Zbarsky [:bzbarsky]]

So is this really blocking 2.0, given comment 10?

Comment 14

[Johnny Stenback (:jst)]

Nope, not blocking.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Based on comment 8, marking this WFM, since it is not
clear to me what could have fixed this on 3.6.
Please reopen if you still see this on trunk.