This is a spin-off from bug #462243 addressing the problem of heuristic query-freshness.

Fixes this particular issue on mnot's webpage.

Comment on attachment 352298 [details] [diff] [review] V1.0 patch w/ xpcshell testcase Changed reviewer

Comment on attachment 352298 [details] [diff] [review] V1.0 patch w/ xpcshell testcase Changed reviewer again...

Hrm, I'd suggest fixing this by making MustValidate() return true if the URI contains a query but no explicit expiration info, except that that function doesn't have access to the query.

Comment on attachment 352298 [details] [diff] [review] V1.0 patch w/ xpcshell testcase + nsresult res = mCachedResponseHead->GetExpiresValue(&tmp); why not use the "rv" variable that exists already? + if (!doValidation && mRequestHead.Method() == nsHttp::Get && index(mSpec.get(),'?')) This is wrong, mSpec can contain #? in which case you don't want to invalidate the cache entry. Use mURI->QueryInterface(nsIURL)->GetQuery? Also it looks like not all lines are below 80 characters, please fix (also in the test) some of my style comments in bug 203271 apply to the test in this bug as well

Same approach, but using GetQuery() instead which is most likely more robust. (In reply to comment #4) > Hrm, I'd suggest fixing this by making MustValidate() return true if the URI > contains a query but no explicit expiration info, except that that function > doesn't have access to the query. I agree that this logic belongs to MustValidate(). It is only called from nsHttpChannel and we could parametrize the method to get access to the query. Your call... :)

Comment on attachment 366655 [details] [diff] [review] V1.1 honoring comments from reviewer + nsCAutoString tmpString; query or queryString would be a more descriptive name for this variable :-)

(In reply to comment #7) > (From update of attachment 366655 [details] [diff] [review]) > + nsCAutoString tmpString; > > query or queryString would be a more descriptive name for this variable :-) I considered calling it "grwwwmPZPX" but decided against it. I guess your suggestion is better... ;)

Comment on attachment 366852 [details] [diff] [review] V1.2 re-naming string-variable [Checkin: Comment 9] http://hg.mozilla.org/mozilla-central/rev/01987cfc569b

An annoying side effect since this change: bug 490095

The trouble with this change (causing bug 490095) is that it overrides the VALIDATE_NEVER flag (i.e. an HTTP request with validation could still be issued even when the flag is set). That flag is set when you go back (http://www.google.com/codesearch/p?hl=en#e_ObwTAVPyo/docshell/base/nsDocShell.cpp&q=VALIDATE_NEVER&exact_package=http://hg.mozilla.org/index.cgi/mozilla-central&l=7590). Putting that logic in MustValidate() is one way to fix that issue.

The fix to this bug has caused the regression in bug 538892.