Closed Bug 99638 Opened 21 years ago Closed 20 years ago

form post data not reposted when using back button

Categories

(Core :: DOM: Navigation, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla0.9.7

People

(Reporter: mozilla.org, Assigned: radha)

References

()

Details

Attachments

(1 file, 3 obsolete files)

I have a saved Bugzilla query called "Mac OS X bugs" which I access using the URL:

http://bugzilla.mozilla.org/buglist.cgi?&cmdtype=runnamed&namedcmd=Mac%20OS%20X%20bugs

Sometimes, when going back to the bug list using the back button, Mozilla drops
the query parameters and just runs

http://bugzilla.mozilla.org/buglist.cgi

I haven't figured out how to reproduce this.
The above applies with build 2001091405 on Mac OS X 10.0.4.
Looks like the following procedure will do it:

1. Log into Bugzilla and store your Bugzilla account login information in
Password manager.
2. Log out of Bugzilla.
3. access a saved query with a URL like

http://bugzilla.mozilla.org/buglist.cgi?&cmdtype=runnamed&namedcmd=xxxx
substituting a saved Bugzilla query for "xxxx"
4. Bugzilla shows a login page, and Password Manager fills in your login
information.
5. Login.
6. Mozilla posts the form data to http://bugzilla.mozilla.org/buglist.cgi and
gives you the results of the named query. Click on a link, and then click the
"back" button.

what should happen:
Mozilla should access http://bugzilla.mozilla.org/buglist.cgi, reposting the
form data.

what happens:
Mozilla accesses http://bugzilla.mozilla.org/buglist.cgi without reposting the
form data, resulting in a query for all bugs.
Can anyone reproduce this on non-Fizzilla builds?
Summary: query parameters sometimes dropped when using back button → query parameters dropped when using back button
As far as I can tell, the problem here is that Mozilla is not reposting the form
data when it returns to the page via the back button.
Summary: query parameters dropped when using back button → form post data not reposted when using back button
Confirmed using Fizzilla/2001091313.
Status: UNCONFIRMED → NEW
Ever confirmed: true
I can not reproduce this in my recent windows build. is this specific to Mac? is
this specific to fizzilla builds?
Blocks: 102998
Keywords: mozilla0.9.7
I suffer from this problem CONSISTENTLY on my personal web site. I log in to my
site and maintain a session ID in the query string.  My site uses frames and
TARGET="_top_" to manipulate the window.  If I click on such a link that takes
over the window and click back, I lose my login query string.  Additionally, If
I clicked that link from a non-default page in a frame (ie. not the page
specified in the frameset), it does not return to that page either, but only the
default page. Additionally when I log in, I cannot click back through pages in a
frame...  Perhaps I should create a login on my site for you to tinker with:

http://kuipers.dhs.org:1333/

Click the Links link. Click Back. It works.

Click on the Family link and enter the following login info:

moztest
M0ZqaT35T

Now click one of the Links icon on the left. Try clicking back. For me it
doesn't work.

Notice the query string. On the Links page, pick Slashdot. Click Back. Notice
the query string is gone and the Links page is also gone (it's back to the
default page).

This is on Mandrake Linux 8.0, Mozilla 0.9.5 (build 2001101202).

I have always had touch and go luck with navigating frames on my site using
back.  0.9.5 introduced the elimination of the querystring.
The login id and password that fred@kuipers.dhs.org don't work.
It does... work, just tried it. However, I realized that the zero in the second
position of the password may have been mistaken for a big-O.

FK
I see the problem. Shall take a look. 
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.6
re-summarising, to reflect the latest problem described here. I couldn't
reproduce the earlier problem describd here. Nor has the reporter provided steps
to reproduce.
Summary: form post data not reposted when using back button → BAck/forward not working well in some frameset pages
The frameset problem belongs in another bug.

There are steps to reproduce this problem in my comment of 2001-09-14 11:14. The
steps have been verified; see the comment by Greg Kolanek 2001-09-18 22:40. This
may be a Mac OS or Mac OS X only bug.
Summary: BAck/forward not working well in some frameset pages → form post data not reposted when using back button
Attached patch Initial patch (obsolete) — Splinter Review
patch for the problem described by Fred in his website. 
This bug (the original bug) as reproduced by james l. in his 20010914 comments
is still reproducible in 2001103108 builds on ALL platforms. uhh - actually
except Linux build 2001110112, I'm looking into that.

You can create/access links like the ones needed here by 'remembering' a query
on the bugzilla query page and then adding the query to your page footer in your
bugzilla prefs - if you don;t already have some. Then bookmarking that link will
give you a BM link in the above mentioned format.
OS: MacOS X → All
Hardware: Macintosh → All
as it turns out thanks to a different linux only bug, you just can't see this
bug on linux.
The same problem also happens going forward - if you have a cgi that uses a
"Post" methodology to present different screens to the user in a sequence, the
first and second page show properly.  When you submit the second page, the 3rd
page is processed, but the same 2nd page is displayed.  If "Reload" is pressed
the proper page is displayed.  This is seen with 0.9.5 milestone release version
and the nightly build from 2001/10/30 (downloaded on 2001/10/31).
cc'ing darin, gagan.

Following the original steps to reproduce, this is what I see.

1. Logout of buzilla and click on a bookmark with url 
http://bugzilla.mozilla.org/buglist.cgi?&cmdtype=runnamed&namedcmd=Mac%20OS%20X%
20bugs.

2. Bugzilla realises that I have logged out and shows me the login page with the 
above url.
3. I fill in the bugzilla login information and click OK. At this time docshell 
creates a nsIHTTPChannel, for the uri, 
http://bugzilla.mozilla.org/buglist.cgi, sets the postdata and sends it to necko 
for loading.
4. However, When nsDocShell::OnNewURI() is called through onStartRequest() 
notification, the channel passed is a nsPartChannel and doesn't directly hold 
the postdata that was originally set on the channel.  nsPartChannel however has 
a member called mMultiPartChannel which holds the original channel and the 
original postdata. 

I think, in  step 4, eventhough necko loads just buglist.cgi without the 
original query parameters, it manages to pass on the original query through this 
nsPartChannel. But this information is not reflected in docshell and session 
history.  Since docshell doesn't find the postdata in the nsPartChannel, it 
stores just http://bugzilla.mozilla.org/buglist.cgi in SH and there by the 
misbehavior happens when the user tries to go back or even reload that page. 

Should docshell look in to nsPartChannel for the postdata? Also, I'm not sure 
that just retrieving  the postdata from nsPartChannel will fix this problem.
radha: i've always wondered why this problem hadn't already been hit.  it seems
odd that in only the multipart stream case, we effectively block access to the
underlying channel.  i have some ideas on how to clean this up, but i haven't
fully sorted them out.

what might make the most sense right now would be to provide a
nsIMultipartChannel or nsIMultipartRequest with an attribute that reveals the
underlying channel.  this would enable docshell to access the real channel. 
once you have the real channel, you should be good to go.

perhaps you'd need to hack something together to test this fix... though it
sounds very much like it could be what's needed.
Attachment #55668 - Attachment is obsolete: true
Attached patch Patch 1.1 (obsolete) — Splinter Review
The latest patch fixes the original problem reported in this bug with bugzilla.
Comment on attachment 57128 [details] [diff] [review]
Patch 1.1

>#include "nsISupports.idl"
>
>interface nsIChannel;
>
>/**
> * An interface to access the the original channel 
> *associated with a MultiPartChannel.
> */
>
>[scriptable, uuid(62d77f66-8ad0-4a7f-91a1-bb048b136490)]
>interface nsIMultiPartChannel : nsISupports
>{
>
>    /**
>     * The request method is case insensitive
>     */
>    readonly attribute nsIChannel originalChannel;
>
>};

some nits:
1- cleanup the whitespace
2- what does "The request method is case insensitive" mean in this context?


>Index: Makefile.in

>-	        nsIByteRangeRequest.idl \
>+    nsIByteRangeRequest.idl \
>+    nsIMultiPartChannel.idl    \
> 		$(NULL)

clean up tabbing.


>Index: makefile.win

> 	.\nsISecurityManagerComponent.idl	\
>     .\nsICachingChannel.idl             \
>     .\nsIByteRangeRequest.idl           \
>+    .\nsIMultiPartChannel.idl  \
> 	$(NULL)

clean up tabbing

>Index: nsMultiMixedConv.cpp

>+NS_IMETHODIMP
>+nsPartChannel::GetOriginalChannel(nsIChannel ** aReturn)
>+{
>+    NS_ENSURE_ARG_POINTER(aReturn);
> 
>+    *aReturn = mMultipartChannel;
>+    NS_IF_ADDREF(*aReturn);
>+}
> 

needs a return NS_OK.



>Index: nsDocShell.cpp

>@@ -5121,7 +5130,14 @@
>             cacheChannel->GetCacheToken(getter_AddRefs(cacheToken));
>         }
>         nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(aChannel));
>-
>+        if (!httpChannel) {
>+            nsCOMPtr<nsIMultiPartChannel>  multiPartChannel(do_QueryInterface(aChannel));
>+            if (multiPartChannel) {
>+                nsCOMPtr<nsIChannel> originalChannel;
>+                multiPartChannel->GetOriginalChannel(getter_AddRefs(originalChannel));
>+                httpChannel = do_QueryInterface(originalChannel);
>+            }
>+        }

how about a helper function for this?  something like GetHttpChannel(),
perhaps?
Attachment #57128 - Flags: needs-work+
Attached patch Updated patch (obsolete) — Splinter Review
Attachment #57128 - Attachment is obsolete: true
Updated patch to reflect suggestions made by darin. W.r.t the changes to
nsDocShell.cpp, I'm not sure if the first set of changes are necessary,( though
it doesn't hurt to be there.) If  I find that it is necessary to have those
changes, I will add a convinience routine. 
Target Milestone: mozilla0.9.6 → mozilla0.9.7
these changes seem fine to me...  it seems that this case is just one of 
(possibly) many where a channel needs to be "wrapped" by another channel for 
some reason...

in the past we have talked about having necko support some kind of 'dynamic 
aggregation' where a channel could have a settable 'outer' channel...

if we are going down the path of having an explicit interface to return the 
'inner' channel, then perhaps we may want a more generic name for the interface 
than 'nsIMultipartChannel'...

of course we don't really have to resolve this before we freeze the public necko 
APIs :-)

-- rick
rick: funny cuz i was just thinking about doing the same thing... see my
comments in bug 90355 :)
Darin,  I think you have put a wrong bug # in your previous comments. 
whoops!  yeah, i meant bug 93055 ;)
Any update on this patch?
Comment on attachment 57286 [details] [diff] [review]
Updated patch

i'm not sure originalChannel is the best name.	how about baseChannel?	the
problem with originalChannel is that the multipart channel is actually longer
lived than the underlying channel(s).
Attachment #57286 - Attachment is obsolete: true
Comment on attachment 58617 [details] [diff] [review]
Updated per darin's comments

radha: sorry for not seeing this before, but GetHttpChannel seems poorly named.
 you first QI from nsIChannel to nsIHttpChannel, and only call GetHttpChannel
if the QI fails.  perhaps the QI step should be included in the GetHttpChannel
method?
Attachment #58617 - Flags: needs-work+
Darin, I thought about it and decided against it for 2 reasons:

1) More than 90% of the loads will succeed in the first QI  and will not need a
call in to the function.
2) Even if I move the QI in to the function, I have to do an additional AddRef
there before returning. Eventually it doesn't matter. 
Comment on attachment 58617 [details] [diff] [review]
Updated per darin's comments

r/sr=darin (but i still think that the name GetHttpChannel is somewhat
confusing)
Attachment #58617 - Flags: needs-work+ → review+
fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
mass-verifying claudius' Fixed bugs which haven't changed since 2001.12.31.

if you think this particular bug is not fixed, please make sure of the following
before reopening:

a. retest with a *recent* trunk build.
b. query bugzilla to see if there's an existing, open bug (new, reopened,
assigned) that covers your issue.
c. if this does need to be reopened, make sure there are specific steps to
reproduce (unless already provided and up-to-date).

thanks!

[set your search string in mail to "AmbassadorKoshNaranek" to filter out these
messages.]
Status: RESOLVED → VERIFIED
Component: History: Session → Document Navigation
QA Contact: claudius → docshell
You need to log in before you can comment on or make changes to this bug.