View source, when invoked for a page with post data (generally not cached), results in a HTTP GET request. The fix for this is being discussed in bug 40867. In lieu of the complete fix, we may need to disable view source for pages with post data on the branch.

We'll take this, we need something immediately. Please get the patch & reviews and checkin ASAP.

-> blake

cc: paw. Can you get someone from Netscape QA to verify once fixed? I'm not sure who tests View Source.

To test, type something in the textfield in the testcase I attached and press Submit Query. The resulting page has post data. The Send Page and View Page Source items (in both the context menu and the View menu) should be disabled. Ctrl+U should also not bring up page source on that page; as far as I can tell, Send Page does not have a keyboard shortcut. The resulting page contains a counter to show how many times that page has been posted to. For the record, I did *not* see the counter increase artificially when I tried viewing the page source a bunch of times (before my patch). I assume someone has actually ensured that a problem exists?

function BrowserViewSourceOfURL(url, charset) { + var isPostData = false; + var webNav = getWebNavigation(); + if (webNav) + isPostData = webNav.postData; + + if (isPostData) return; Shouldn't this code live in "function BrowserViewSource()" instead? BrowserViewSourceOfURL is called if view-source: urls are typed in the URL bar as well, and then you would be doing _completely_ the wrong thing (preventing a source load because a completely unrelated page has post data). Also, BrowserViewSourceOfURL() is called to view frame source. So this will disable view frame source for cases when the frameset itself (but not the frame) is the result of a POST... +function UpdateNecessaryItems(eltId) This could really use a better function name.... Otherwise in the XUL it's not clear why it's being called on cmd_sendPage and View:PageSource I assume a fix for view frame source is pending a nice way to get a frame docshell? If we really need it for rtm I have a function that takes the root docshell and an nsIDOMDocument and walks the docshell tree till it finds the docshell that belongs to that document. And yes, I was testing the whole "view source of POST pages" thing just 5 days ago and the bug was certainly present and thriving....

r=bbaetz. this won't stop view frame source though.

bz: yes, we'll try to come up with a frames solution. I'd probably prefer not to use the method you describe for fear of slowing down context menu creation a lot. Although no one else around here as a better idea. Are you able to reproduce the problem on this page, specifically? Ben says "sr=ben"

r=bzbarsky The behavior I saw with the testcase you attached: 1) Load form 2) submit it. Request id is 411 3) view source. Request id is 412 4) close view source. View source again. Id is still 412 (it's picking up the view-source:whatever url from cache this time). So yes, I was seeing the bug in question. :) Also, try http://www.zbarsky.org/~bzbarsky/sourcetest.html if you wish. That will make it very obvious when a reload happens -- content on page and content in view source will be different. > I'd probably prefer not to use the method you describe for fear of slowing > down context menu creation a lot. I agree.... we need a better way of handling that. I forgot that this code is running on each context menu creation (what I have in my tree only runs when "view frame source" is actually selected).

Fix is in.

Terri can you verify this bug. Thanks.

Fix for this has caused the following two regressions with context menus. There may be more which we have not seen: http://bugzilla.mozilla.org/show_bug.cgi?id=91719 http://bugzilla.mozilla.org/show_bug.cgi?id=91720

What about 'File->Edit Page...'. That triggers another POST to be sent.

Argh. Didn't mean to verify, meant to reopen.

Can we get r= and sr= on the second patch, in case we decide to respin for this?

r=bbaetz on http://bugzilla.mozilla.org/showattachment.cgi?attach_id=43267

sr=hewitt on that last patch

I guess this is now irrelevant, or something...

Hrm. I feel bad about stirring this hive, but ... don't we want this same stuff on the 0_9_4 branch? (i.e., it was thought to be needed for 0_9_2).

Pchen - Do we need this on the branch?

can we discuss this one @ today's PDT meeting?

PDT+, let's get this into the 094 branch, after the diff is updated against the current branch, and you get new reviews.

I had a look at updating this to the 9.4 branch, but I can't find where I'm supposed to find |postData| in the current branch. jag?

Okay, the backend isn't on the 094 branch either, then. This patch needs to go in as well: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=42683. It is the same one that we used for 092/6.1. Is PDT okay with that?

The PDT is okay with whatever it takes to get this done, so long as the diff(s) are updated against the current branch, and that we get new reviews. If this is done, and we feel confident, then pls go ahead and check it into the 094 branch. btw - i think we owe blake a pizza or 2. :-)

blaker - we'd like to have candidate in hand by monday, so it would be sweet, if you got this in tonight. thanks ...

okay, fixed.

Verified fixed w2k build 2001-10-15-05-0.9.4