Per the API review, nsIWebNavigation::LoadURI() needs to be able to bypass the session history. This functionality would be reflected in nsIDocShell's LoadURI as well. Radha has pointed out two ways of doing this: 1. a bool flag in the api, say "aBypassHistory". 2. an additional loadType, say "bypassHistory". The latter begs the question of how many times are we going to want to extend the load type, *and* should the loadType be an &'able bit field so you can compound loadtypes? IMO we should make the load type a bit field and extend it to include the bypassing of history.

In the first case (ie., passing a bool flag aBypassHistory), it is not enough we just pass a bool flag, we probably have to have a member in docshell which will hold the value and when uriloader calls in to OnNewUri() we check on this member to decide whether we need to add the url to history.

I think we should separate the load flags from the load type. We add a new load flags parameter to loadURI() and reload(). The flags allow the load action to bypass the history, proxy & session history and anything else we dream up. Some combinations of load flags & load command will be nonsensical to reload() so these will have to be rejected.

PDT thinks is a P3 for nsbeta3, setting priority and status whiteboard to reflect that. Would be nice if SH worked better though :-)

back to P1. we need this in for embedding api reasons.

I'm not sure I see why ou need this for embedding reasons? Why is it so important? How is the person loading the url going to really know if they want the url in session history or not? I bet they really aren't. I just checked in a fix with radha to make sure entries aren't added to session history unless the url resulted in a document getting loaded. I bet this change fixes what you were really after here. But maybe I'm wrong. Is it still P1 after my checkin?

One situation where browser can use this feature is, load a page & let it complete. Go to urlbar and press <Enter>. Right now we load the page again *and* add it to SH. This second entry is not really needed (though it doesn't hurt). This can be avoided when the browser knows that the urls are the same and there by let docshell know thro' LoadURI() that the url need not be added to SH. We do not want to do the same checks in docshell due to complications with postdata comparisons. See bug 49913 for more details. This probably doesn't have to be a p1 (unless embedding team has other goals that are met by implementing this)

This is getting messy! Both the nsIDocShell and nsIDocShellLoadInfo interfaces declare the same load types, loadNormal, loadReloadNormal etc. It doesn't appear the nsIDocShell enums are used at all or only internally to nsDocShell.cpp. The nsIDocShellLoadInfo stuff is for session history support. So some questions: If we're replacing the load types, do the new things get put on nsIDocShell or nsIDocShellLoadInfo? The LoadURI already accepts an nsIDocShellLoadInfo interface as the second parameter. Should the load flags be an additional parameter or an attribute of the loadinfo? Does the session history need to store any of these flags? Do flags such as bypass proxy, bypass cache need to be stored with the load info in the session history?

Sounds like nsIDocShell enums need to go away in favor of the load info enums. I'd say they should be an attribute on the loadinfo. I don't know the answer to your last Q. Radha?

Neither the load flags nor the load Info need to be stored in SH. All loads initiated by SH are loadHistory and they get mapped to nsIChannel::VALIDATE_NEVER. I believe this is the right behavior.

There are several places where the load info load type is set to something else: http://lxr.mozilla.org/seamonkey/search?string=setloadtype That means the load type is still needed as part of the load info object. Some of the load types used would also become flags in the new scheme. If we don't save flags with the load info, is there any chance that reloading the page from history later on will get it wrong?

To clarify what I'm doing. The following load types will be put into nsIDocShellLoadInfo. const unsigned long LOAD_TYPE_NORMAL = 1; // Normal loading const unsigned long LOAD_TYPE_RELOAD = 2; // Reload const unsigned long LOAD_TYPE_HISTORY = 3; // Load from history The following load flags will be added to nsIWebNavigation. const unsigned long LOAD_FLAGS_IS_REFRESH = 0x0001; // Refresh timer const unsigned long LOAD_FLAGS_IS_LINK = 0x0002; // Link click const unsigned long LOAD_FLAGS_BYPASS_CACHE = 0x0004; // Bypass the cache const unsigned long LOAD_FLAGS_BYPASS_PROXY = 0x0008; // Bypass the proxy const unsigned long LOAD_FLAGS_BYPASS_HISTORY = 0x0010; // Bypass the history const unsigned long LOAD_FLAGS_REPLACE_HISTORY = 0x0020; // Replace the history entry The LoadURI & Reload methods in nsIWebNavigation and nsIDocShell will get a new flags parameter. The nsDocShell class will be modified to test for flags to modify the loading/history behaviour - this is the most fraught part!

I think the flags are required along with the load types. Can there be a case of no load flag? ie., if I type in something in the urlbar. It is just a loadNormal and none of the laodFlags listed below are required. What about loadNormalReplace case? Will it be a load flag or type

loadNormalReplace would be LOAD_TYPE_NORMAL with the flag LOAD_FLAGS_REPLACE_HISTORY. If the flags are 0 then LOAD_TYPE_NORMAL would behave just like loadNormal. All the mappings: loadNormal -> LOAD_TYPE_NORMAL, 0 (or some constant) loadNormalReplace -> LOAD_TYPE_NORMAL, LOAD_FLAGS_REPLACE_HISTORY loadHistory -> LOAD_TYPE_HISTORY, 0 loadReloadNormal -> LOAD_TYPE_RELOAD, 0 loadReloadBypassCache -> LOAD_TYPE_RELOAD, LOAD_FLAGS_BYPASS_CACHE loadReloadBypassProxy -> LOAD_TYPE_RELOAD, LOAD_FLAGS_BYPASS_PROXY loadReloadBypassProxyAndCache -> LOAD_TYPE_RELOAD, LOAD_FLAGS_BYPASS_CACHE | LOAD_FLAGS_BYPASS_PROXY loadLink -> LOAD_TYPE_NORMAL, LOAD_FLAGS_IS_LINK loadRefresh -> LOAD_TYPE_NORMAL, LOAD_FLAGS_IS_REFRESH The new LOAD_FLAGS_BYPASS_HISTORY is used to bypass the session history. Other combinations should be rejected.

Bill is interested in this..

Can you explain the mapping from the nsIWebNavigation.idl load flag constants to nsIChannel (and nsIHTTPChannel) load flags (actually, they call 'em "load attributes")? What I have to know (to fix bug 6119) is what flag values to pass in to ensure that nsIChannel::VALIDATE_NEVER gets used?

The patch doesn't let you specify VALIDATE_NEVER but it does make it easier for future modifications to do that. Now that LoadURI has a flags parameter, you could make up a new flag that did just that.

Radha's comment on 09/05 implied that session history loads would all result in use of VALIDATE_NEVER. I just wanted to verify that. Perhaps there should be *two* flags on nsIWebNavigation::loadURI. One to control session (and global?) history, the other to be passed on to the channel (these would just be "load attributes" as defined by nsIChannel. I can see how the channel doesn't need to deal with history, but it seems the history control stuff should be built on top of what's there for the channel. Otherwise, one has to grovel through the implementation to figure out what's being passed to the channel.

Bill, LoadHistory eventually gets mapped to VALIDATE_NEVER in nsDocShell::DoChannelLoad(). I think this takes care of your concern.

Diffs checked in

Radha, I'm not sure what "loadHistory" means. To all: What do I pass as the second argument on nsIWebNavigation::LoadURI in order to get VALIDATE_NEVER passed to the channel? Or, to put it another way, how do I change viewSource.js so it fixes bug 6119?

second argument to LoadURI() is of type nsIDocShellLoadInfo. One of the element sin nsIDocShellLoadInfo is loadType. You need to do something very similar to this, to set a loadtype of LOAD_HISTORY. mRootDocShell->CreateLoadInfo (getter_AddRefs(loadInfo)); // This is not available yet loadInfo->SetLoadType(nsIDocShellLoadInfo::loadHistory); loadInfo->SetSHEntry(nextEntry); // Time to initiate a document load return docShell->LoadURI(nexturi,loadInfo, nsIWebNavigation::LOAD_FLAGS_NONE); nsIDocShellLoadInfo::loadHistory gets mapped to the enum LOAD_HISTORY in nsDocShell.cpp and later gets mapped to nsIChannel::VALIDATE_NEVER

Hm. viewSource.js is currently calling nsIWebNavigation::LoadURI, whose second argument is of type PRInt32 (nsIWebnavigation load type flags). Must I switch to do the load via the docshell? I didn't understand why it was using the nsIWebNavigation interface; I figured there must have been a good reason, though. Does anybody know?

verified fixed (notwithstanding any subsidiary open questions (at the end of this bug report)).

I'm not sure what the issue is with viewsource.js but the only reason you should use nsIDocShell::LoadURI as opposed to nsIWebNavigation::LoadURI is if you are doing session history stuff or you want to pass in some load info such as postdata. Otherwise use nsIWebNavigation.

OK, view source needs to provide post data and specify VALIDATE_NEVER, so I guess nsIDocShell::LoadURI is the way to go.