As mentioned in bug 442806, the LOAD_CHECK_OFFLINE_CACHE flag is passed along after a redirect. This could lead to the LOAD_CHECK_OFFLINE_CACHE flag being passed to a load that does not have the offline-app permission.

This adds to nsDocShell (the same class that sets LOAD_CHECK_OFFLINE_CACHE flag during initial load) check for new URI having offline application permission on redirect.

Comment on attachment 336392 [details] [diff] [review] v1 I'm not at all sure that this notification fires at the right time to do this (or at least is guaranteed to do so). Hooking into the actual "we're setting up the redirect channel" notification seems like it would be better, but really Christian is the one who knows about this stuff...

(In reply to comment #2) > (From update of attachment 336392 [details] [diff] [review]) > I'm not at all sure that this notification fires at the right time to do this > (or at least is guaranteed to do so). Hooking into the actual "we're setting > up the redirect channel" notification seems like it would be better, but really > Christian is the one who knows about this stuff... The callback is invoked right from the place "we're setting up the redirect channel". I put the code to the callback - the class - that originally sets the LOAD_CHECK_OFFLINE_CACHE on the original channel. As Dave Camp said, nsDocShell has to take care of this flag, not the channel itself.

(In reply to comment #2) > (From update of attachment 336392 [details] [diff] [review]) > I'm not at all sure that this notification fires at the right time to do this > (or at least is guaranteed to do so). Hooking into the actual "we're setting > up the redirect channel" notification seems like it would be better, but really > Christian is the one who knows about this stuff... Yeah, OnChannelRedirect directly calls this. This should be fine.

Comment on attachment 336392 [details] [diff] [review] v1 + aNewChannel->SetLoadFlags + (loadFlags & ~nsICachingChannel::LOAD_CHECK_OFFLINE_CACHE); I think more common style is to put the ( on the previous line

I wonder if it would be a safer contract to have the channel not propagate LOAD_CHECK_OFFLINE_CACHE by default, and require docshell to set it appropriately in its redirect handler.

That might not be a bad idea.

This fix (not propagating LOAD_CHECK_OFFLINE_CACHE on a redirect) was included as part of the patch for 445544.