When assign window.location or simply using the address bar in tab where an offline application is running (i.e. a document with associated application cache) navigation fails ("we are in offline mode" page is shown, see bug 443450. This happens in case the new URL we want to navigate to is not contained in the application cache associated with the document (most cases). The reason is the document is still assigned an application cache even after the location is changed and the underlying channel loads only from the application cache gained from the above document (what is correct!). We must remove the application cache from the document on the right place under the correct condition. Probably nsLocation::Assign is the right place? Or somewhere above in docshell where it is more easy to recognize the document is top level?

So it looks like what's happening here is that the http channel for the new location is asking the docshell (via the loadgroup) for the current application cache. Until SetupNewViewer() is called, the docshell is still returning the current toplevel's doc to satisfy the request. The attached patch adds a load flag to the channel that inhibits inheriting the app cache from the docshell. The docshell passes this flag in to toplevel document loads to prevent them from inheriting the old application cache setting.

This patch works for problem with navigation. But it bypasses cache grab from document when refreshing the page that leads to violation of the spec: when new version of the cache is published the cache is swapped when page is reload and not after the swapCache method is called. This nit can be held as known issue in my opinion. Just a question: isn't testing of LOAD_INITIAL_DOCUMENT_URI flag sufficient?

This is the same as v1 but prevents bypass of the cache when the page is being reloaded. Not sure this is clean solution, but preserves the correct behavior.

Comment on attachment 330044 [details] [diff] [review] v2 I misunderstood the expected behavior: on reload the document must use the newer cache, so v1 patch is correct and sufficient solution to this issue.

New version uses a new interface (nsIApplicationCacheChannel) rather than a new load flag. This interface also includes an attribute for the equivalent of LOAD_CHECK_OFFLINE_CACHE. I also included a fix for 450175, as this patch adds a new attribute to potentially be propagated. LOAD_CHECK_OFFLINE_CACHE shouldn't really be needed anymore, and we could remove it to get an extra flag back. I can either include that in this patch or file a followup.

Comment on attachment 343622 [details] [diff] [review] new patch Why not have the new interface inherit from nsIApplicationCacheContainer? > I can either include that in this patch or file a followup. Followup is fine. The rest looks fine to me; I'd like Honza to double-check the actual cache logic and test.

Comment on attachment 343622 [details] [diff] [review] new patch This conforms the previous version of the spec. Looks ok, I would just expect (root == this) check - check for top level document - is present also in OnRedirectStateChange before chooseAppCache is set. And yes, we should remove the LOAD_CHECK_OFFLINE_CACHE flag. It is now useless.

(In reply to comment #7) > (From update of attachment 343622 [details] [diff] [review]) > This conforms the previous version of the spec. Looks ok, I would just expect > (root == this) check - check for top level document - is present also in > OnRedirectStateChange before chooseAppCache is set. ShouldCheckAppCache() does the (root == this) check. LoadURI() does it also because it needs to set inheritApplicationCache, but that's copied to the new channel during redirect.

interdiff of the review request fix (deriving nsIApplicationCacheChannel from nsIApplicationCacheContainer).