39668 - Need a way to get a handle to postdata

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P3)

Description

[Radha on family leave (not reading bugmail)]

Session History needs to get a handle to  postdata and save it in its cache. 
But Postdata is not persisted in docshell. SH needs some kind of mechanism to 
get hold of it from docshell.

Comment 1

[locka]

This is some explanatory ramblings about the docshell and the postdata. I'm 
writing them down so I remember how things are done myself as much by way of 
explanation!

The last place where postdata is currently known by the nsDocShell is in the 
DoURILoad method.

http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#2518

The method creates a channel and in the case of an HTTP channel, sets the post 
data stream on it. The docshell then initiates URI loading and forgets all about 
the postdata.

Later on when the URI begins to load asynchronously. Docshell is called by the 
docshell's content listener to handle the incoming content. It is here that the 
docshell adds a new entry to the session history. Currently it only adds a URI 
to the session history and a fake, empty stream where the postdata should be.

http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDSURIContentListener.cpp
#81
http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#2636

Fortunately, you should see from the links, that the one parameter passed to 
nsDocShell::OnLoadingSite is the channel that the postdata was originally set 
on. This means it should be possible to call GetUploadStream on the channel 
(assuming it's HTTP of course) and extract out what was put in and add it to the 
session history. So hopefully all I need add is a new parameter to 
nsDocShell::AddToSessionHistory (either the channel or the postdata stream will 
do) and use that instread of the fake stream.

I will check to see tomorrow if the docshell takes notice of the postdata stream 
when it loads a URI from a session entry. Hopefully it does in which case, this 
all may be possible to fix in less than 15 lines of code.

Comment 2

[locka]

OK, I've tried out the postdata stuff and it works. Docshell gives the correct 
postdata to the session history and it's used in back/forward. I just wonder now 
if we should always repost data or whether we should prompt the user on whether 
to repost it like 4.x does? If it's the latter case, is there some XUL to do 
this already?

Comment 3

[Radha on family leave (not reading bugmail)]

Adam, this is good news. I believe there is no XUL to it.  May be I can come up 
with one on Monday.

Comment 4

[Radha on family leave (not reading bugmail)]

It looks like a dialog that says, "Repost data?" can be put up using the 
nsIprompt service. There are some examples in, 
http://lxr.mozilla.org/seamonkey/source/editor/base/nsEditorShell.cpp#2862
http://lxr.mozilla.org/seamonkey/source/extensions/wallet/src/wallet.cpp#758

The return value from the confirm() , can be used to decide whether the postdata 
s'd be passed on to the url or not.

Comment 5

[locka]

We still need a DTD for the dialog with the locale specific strings in it and 
only the wallet example you gave does this and it's quite complicated. The 
editorshell example, doesn't do this and appears to be an unused method anyway.

For the time being I've checked in the promptless version of the postdata code 
for you to look at. It will always resubmit the postdata when the back 
button is pressed. I'll investigate how to display the dialog to the user as the 
next thing I do. IE seems to have an interesting way of doing it. If you hit the 
back button, it loads the page from cache, but if you then hit refresh it asks 
you if you want to repost the data again. This sounds more sensible to me than 
asking them everytime they hit the back button.

Comment 6

[Radha on family leave (not reading bugmail)]

Attachment: docshell.cpp diffs

Comment 7

[Radha on family leave (not reading bugmail)]

Attachment: diffs for appstrings.properties

Comment 8

[Radha on family leave (not reading bugmail)]

adam, please review the attached diffs. This brings up the dialog, before 
reposting the data.

Comment 9

[locka]

Comments on the attached code.

The code looks fine in general though tf we fail to get the string data, should 
we default to post the data instead of failing? I think some embedding apps will 
probably not want to display a dialog here and the easiest way not to is to not 
include the language strings.

The text says "Repost from data?" not "Repost form data?". Should we also 
consider making this message more meaningful as well so it explains that the 
data needs to be resubmitted to the server?

Comment 10

[Radha on family leave (not reading bugmail)]

adam, I had the concerns that you have mentioned. I think the code can be 
changed so that if for somereason, you don't get the string data, you stil 
repost. Also, I noticed that 4.x, doesn't always ask "Repost form data?" for all 
pages that have post data in them. I don't know what is the criteria in 4.x to 
put up that dialog. If you follow the amazon.com example in  
http://bugzilla.mozilla.org/show_bug.cgi?id=1718, 4.x doesn't ask "Repost form 
data?" when going back. I think it will be good to find out. cc'ing few people.

I'm with you in changing the wording too. 

Comment 11

[Hixie (not reading bugmail)]

word change: Yes, especially now that e-commerce is more widespread, some 
friendly mention that "reposting form data" might mean that the order is sent
again would not go amiss...

prompting: Could that have something to do with the page being cached? Does 4.x
ever cache a POST page? If so, then it would not ask to redisplay a page that
had not yet expired, right?

Comment 12

[locka]

Perhaps 4.x shows the previous page unless something in the meta-content says 
the page has expired.

There are definitely situations where you want to ask if the user wants to 
repost data or not. For example, if I were in bugzilla and submitted a new 
comment, I wouldn't want to click back or forward and discover that Mozilla had 
submitted it again for me.

Comment 13

[Radha on family leave (not reading bugmail)]

Adam, I guess I'll checkin my changes incorporating the suggestion you made ie., 
if we fail to get the string for the dialog, we will continue to repost the data 
, instead of failing. I think we can figure the other issues later. 

Comment 14

[locka]

With Radha's changes into the code, I'm marking this fixed