99638 - form post data not reposted when using back button

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[J Luh]

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.

Comment 1

[J Luh]

The above applies with build 2001091405 on Mac OS X 10.0.4.

Comment 2

[J Luh]

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.

Comment 3

[Greg K.]

Can anyone reproduce this on non-Fizzilla builds?

Comment 4

[J Luh]

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.

Comment 5

[Greg K.]

Confirmed using Fizzilla/2001091313.

Comment 6

[Radha on family leave (not reading bugmail)]

I can not reproduce this in my recent windows build. is this specific to Mac? is
this specific to fizzilla builds?

Comment 7

[Fred]

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.

Comment 8

[Radha on family leave (not reading bugmail)]

The login id and password that [e-mail] don't work.

Comment 9

[Fred]

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

Comment 10

[Radha on family leave (not reading bugmail)]

I see the problem. Shall take a look. 

Comment 11

[Radha on family leave (not reading bugmail)]

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.

Comment 12

[J Luh]

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.

Comment 13

[Radha on family leave (not reading bugmail)]

Attachment: Initial patch

Comment 14

[Radha on family leave (not reading bugmail)]

patch for the problem described by Fred in his website. 

Comment 15

[Claudius Gayle]

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.

Comment 16

[Claudius Gayle]

as it turns out thanks to a different linux only bug, you just can't see this
bug on linux.

Comment 17

[s.kumar]

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).

Comment 18

[Radha on family leave (not reading bugmail)]

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.

Comment 19

[Darin Fisher]

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.

Comment 20

[Radha on family leave (not reading bugmail)]

Attachment: Patch 1.1

Comment 21

[Radha on family leave (not reading bugmail)]

The latest patch fixes the original problem reported in this bug with bugzilla.

Comment 22

[Darin Fisher]

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?

Comment 23

[Radha on family leave (not reading bugmail)]

Attachment: Updated patch

Comment 24

[Radha on family leave (not reading bugmail)]

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. 

Comment 25

[rpotts (gone)]

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

Comment 26

[Darin Fisher]

rick: funny cuz i was just thinking about doing the same thing... see my
comments in bug 90355 :)

Comment 27

[Radha on family leave (not reading bugmail)]

Darin,  I think you have put a wrong bug # in your previous comments. 

Comment 28

[Darin Fisher]

whoops!  yeah, i meant bug 93055 ;)

Comment 29

[J Luh]

Any update on this patch?

Comment 30

[Darin Fisher]

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).

Comment 31

[Radha on family leave (not reading bugmail)]

Attachment: Updated per darin's comments

Comment 32

[Darin Fisher]

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?

Comment 33

[Radha on family leave (not reading bugmail)]

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 34

[Darin Fisher]

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)

Comment 35

[Radha on family leave (not reading bugmail)]

fix checked in.

Comment 36

[sairuh (rarely reading bugmail)]

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.]