Closed Bug 50949 Opened 24 years ago Closed 24 years ago

nsIWebNavigation::LoadURI() needs to support session history bypass.

Categories

(Core Graveyard :: Embedding: APIs, defect, P1)

Product:
Core Graveyard
Component:
Embedding: APIs
Platform:
x86
Linux
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: jud, Assigned: adamlock)

References

Details

(Keywords: embed, Whiteboard: [nsbeta3+][PDTP3])

Attachments

(2 files)

LoadURI & Reload changed to use load flags - comments/review please.
26.14 KB, patch
Details | Diff | Splinter Review
Updated context diff. I will email the reviewers list for approval but feel free to comment
60.74 KB, patch
Details | Diff | Splinter Review 
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.
Keywords: embed, nsbeta3
Whiteboard: [nsbeta3+]
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.
Priority: P3 → P1
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.
Status: NEW → ASSIGNED
PDT thinks is a P3 for nsbeta3, setting priority and status whiteboard to
reflect that. Would be nice if SH worked better though :-)
Priority: P1 → P3
Whiteboard: [nsbeta3+] → [nsbeta3+][PDTP3]
back to P1. we need this in for embedding api reasons.
Priority: P3 → P1
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..
Blocks: 6119
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
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
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)).
Status: RESOLVED → VERIFIED
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.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: