Last Comment Bug 331825 - Unable to do conditional gets via XMLHttpRequest with 1.5.0.1, works in 1.0.7
: Unable to do conditional gets via XMLHttpRequest with 1.5.0.1, works in 1.0.7
Status: VERIFIED FIXED
: regression, verified1.8.0.7, verified1.8.1
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: 1.8 Branch
: All All
: -- normal (vote)
: mozilla1.8.1
Assigned To: Darin Fisher
:
Mentors:
http://goolamabbas.org/cget/cget.html
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-03-26 22:45 PST by Yusuf Goolamabbas
Modified: 2007-03-03 08:50 PST (History)
9 users (show)
mtschrep: blocking1.8.1+
dveditz: blocking1.8.0.5-
sciguyryan: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 patch (5.07 KB, patch)
2006-06-21 18:34 PDT, Darin Fisher
bzbarsky: superreview+
Details | Diff | Review
corresponding regression test (2.42 KB, patch)
2006-06-21 18:45 PDT, Darin Fisher
no flags Details | Diff | Review
v1.1 patch (4.38 KB, patch)
2006-06-22 15:15 PDT, Darin Fisher
cbiesinger: review+
dveditz: approval1.8.0.7+
mtschrep: approval1.8.1+
Details | Diff | Review

Description Yusuf Goolamabbas 2006-03-26 22:45:27 PST
Hi, I was trying to write a simple ajaxy application to learn Mochikit. The URL above calls some simple JS functions which get a static file periodically and print the HTTP status response code. 

I use the If-Modified-Since and If-None-Match header fields to simulate a conditional get. The expectation is that the the alert window will show 200 the first time and then 304 subsequently. This behaviour is demonstrated with Internet Explorer 6.0/Windows and Firefox 1.0.7 on MacOSX. However if I use Firefox 1.5.0.1 on Windows 2000/XP and MacOSX I am only shown '200' in the alert box on the first time and then on subsequent call outs.
Comment 1 Yusuf Goolamabbas 2006-03-28 22:18:21 PST
If I change the pref network.http.use-cache from true to false. Firefox behaves as expected. I have changed the component to networking. 
Comment 2 Yusuf Goolamabbas 2006-03-28 22:19:39 PST
sorry for bugspam, really meant networking:cache
Comment 3 Yusuf Goolamabbas 2006-05-08 21:36:58 PDT
I've also filed a corresponding bug in WebKit's bugzilla as well as via Apple's Radar

http://bugzilla.opendarwin.org/show_bug.cgi?id=8210
Comment 4 Boris Zbarsky [:bz] 2006-05-23 14:05:49 PDT
> I have changed the component to networking.

You want to change the assignee when you do that if you expect someone to notice....
Comment 5 Darin Fisher 2006-05-23 14:16:44 PDT
So, what is the intended behavior of XMLHttpRequest?  As I understand it, both the W3C and WhatWG have made efforts to standardize XMLHttpRequest.  Is there a spec for what should happen if these request headers are set?
Comment 6 Hixie (not reading bugmail) 2006-05-24 05:38:58 PDT
anne: please ensure your spec covers this. If it already does, please provide a link to the relevant section. Thanks!
Comment 7 Anne (:annevk) 2006-05-25 03:08:33 PDT
Look for HTTP cache in http://dev.w3.org/cvsweb/~checkout~/2006/webapi/XMLHttpRequest/publish/XMLHttpRequest.html#dfn-send

Please send comments to public-webapi@w3.org.
Comment 8 Darin Fisher 2006-05-25 07:22:35 PDT
OK, well that certainly supports this bug.

-> Core: Networking: HTTP
Comment 9 Daniel Veditz [:dveditz] 2006-06-07 16:12:33 PDT
Would consider a fix for this regression in 1.8.0.5, but it's not going to write itself assigned to nobody@mozilla.org -- anyone actively working on this?
Comment 10 Daniel Veditz [:dveditz] 2006-06-09 14:21:28 PDT
Would need a tested trunk and 1.8 branch patch before we can consider it for the 1.8.0 branch. Not blocking, but if you get such a patch you can ask for approval on it and we'll consider it.
Comment 11 Mike Schroepfer 2006-06-21 10:43:55 PDT
Darin - any thoughts on what the right fix is here?
Comment 12 Anne (:annevk) 2006-06-21 15:08:26 PDT
Draft is now located at http://dev.w3.org/cvsweb/~checkout~/2006/webapi/XMLHttpRequest/Overview.html?content-type=text/html;%20charset=utf-8#dfn-send ... This hasn't changed though.
Comment 13 Darin Fisher 2006-06-21 17:27:35 PDT
I can reproduce the behavior difference  FF1.0 and FF1.5.  I'm investigating to see what changed.
Comment 14 Darin Fisher 2006-06-21 17:41:13 PDT
OK, this bug appears to be caused by the fact that we no longer send 'Pragma: no-cache' and 'Cache-control: no-cache' with each XMLHttpRequest GET.  I didn't realize that Firefox 1.0 did that, but it seems like a good thing that Firefox 1.5 does not.  The 304 is still happening, however, it is just happening transparently to the application.  Necko reports a 304 as a 200 since it is able to re-use its cached copy.  Perhaps it should realize that the consumer is initiating the conditional request and skip trying to use its own cache.
Comment 15 Darin Fisher 2006-06-21 17:56:15 PDT
Here's the old code where the local cache was unconditionally bypassed:
http://lxr.mozilla.org/aviary101branch/source/extensions/xmlextras/base/src/nsXMLHttpRequest.cpp#1543

The change was made in bug 268844.  The fact that conditional queries ever worked in Firefox 1.0 is simply an unintended side-effect of our support for multipart queries (which was added in bug 237319 just before we branched for Firefox 1.0).

I think this bug should be fixed inside Necko's HTTP implementation.
Comment 16 Darin Fisher 2006-06-21 18:34:08 PDT
Created attachment 226581 [details] [diff] [review]
v1 patch
Comment 17 Darin Fisher 2006-06-21 18:45:56 PDT
Created attachment 226583 [details] [diff] [review]
corresponding regression test
Comment 18 Darin Fisher 2006-06-22 15:15:43 PDT
Created attachment 226714 [details] [diff] [review]
v1.1 patch

Here's a slightly better variant on the same patch that biesi suggested.
Comment 19 Darin Fisher 2006-06-22 17:49:47 PDT
fixed-on-trunk
Comment 20 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2006-06-23 10:25:59 PDT
What effects (if any) might this have on stuff other than XMLHttpRequest?
Comment 21 Darin Fisher 2006-06-27 13:27:36 PDT
fixed1.8.1
Comment 22 Darin Fisher 2006-06-27 15:25:24 PDT
Comment on attachment 226714 [details] [diff] [review]
v1.1 patch

It might be worth back-porting this fix to the 1.8.0 branch.
Comment 23 Daniel Veditz [:dveditz] 2006-08-15 15:02:38 PDT
Comment on attachment 226714 [details] [diff] [review]
v1.1 patch

approved for 1.8.0 branch, a=dveditz for drivers
Comment 24 Darin Fisher 2006-08-15 18:48:57 PDT
Grr.. the patch did not apply cleanly to the 1.8.0 branch :-(
Comment 25 Darin Fisher 2006-08-18 14:35:36 PDT
fixed1.8.0.7
Comment 26 alice nodelman [:alice] [:anode] 2006-08-25 15:58:44 PDT
using testcase described in comment #0

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.7pre) Gecko/20060825 Firefox/1.5.0.7pre

verified 1.5.0.7

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1b2) Gecko/20060825 BonEcho/2.0b2

verified 1.8.1b2

Note You need to log in before you can comment on or make changes to this bug.