73905 - CSS/JS documents are not cached.

Status: VERIFIED FIXED

(Core :: Networking: Cache, defect)

Description

[Ben Goodger (use ben at mozilla dot org for email)]

Steps to reproduce:

- have prefs for 'check page for updates' set to 'Never'
- go to a page with static content (e.g. www.mozilla.org)
- go to another page with static content (e.g. go to the 0.8.1 release notes 
page from www.mozilla.org's front page)
- click 'Back'

Actual results:
- some form of network connection made, prolonged throbber activity (perhaps 
it's checking for updates anyway?) then the previous page is loaded immediately 
from cache.

Expected results:
- previous static page loads instantly from cache, without any network activity.

This additional network traffic is unnecessary given this pref setting, and eats 
up a lot of time going back and forward. For comparison, try this in Internet 
Explorer or Netscape 4 and you'll see it behaving correctly.

Comment 1

[Ben Goodger (use ben at mozilla dot org for email)]

nominate for beta as this makes back & forward performance slower than they need 
to be. 

Comment 2

[Ben Goodger (use ben at mozilla dot org for email)]

bbaetz tells me that CSS/JS files are loaded with pragma: nocache. This seems 
wrong... (and seems like it would affect a lot of pages)

Comment 3

[Darin Fisher]

ben: can you confirm that the only network traffic you are seeing is CSS?

i will look for the bug in which CSS caching was disabled...

Comment 4

[Darin Fisher]

i can't find the bug... but there is a spot someplace in layout where the
HTTP request headers are tweaked for CSS downloads to make them non-cacheable.

Comment 5

[Darin Fisher]

jud: if i recall, you wrote the patch to disable CSS caching in the old world...
what was the reason for doing this back then? do you remember the corresponding
bug? thx!

Comment 6

[Judson Valeski]

Let this bug be a lesson to me in not checking in someone else's code. I have no
idea. There was some problem where css on some page wasn't getting used or
something. sorry :-/. I suppose we could turn caching on for these again and see
what happens.

Comment 7

[Darin Fisher]

changing summary

Comment 8

[Darin Fisher]

Attachment: enables caching of CSS documents

Comment 9

[David Hyatt]

I'm confused about CSS, JS, and XBL files.  Here's the behavior I would expect
for a setting of "Once per seesion."

(1) I go to an HTML page and fetch it and all the CSS, JS, and XBL files into my
cache.
(2) At some later point, I go there again.  The HTML page is pulled out of the
cache without checking.  I would expect the CSS, JS, and XBL files also to be
pulled out of the cache without checking.
(3) I hit reload.  Now I should check the server not only for the HTML files but
also for the CSS, JS, and XBL files.

I'm wondering if (3) will still be broken with this patch.  With a load of
NORMAL, in the reload situation, will you still just grab the CSS, JS, and XBL
out of the cache without actually pinging the server?

In the case of CSS, JS, and XBL, it seems that they should inherit load
attributes from the load attributes for the parent HTML page.  Or am I missing
something here?

Comment 10

[Darin Fisher]

they should inherit load attributes from the parent page automagically when
they are added to the load group for the page.  the settings in nsCSSLoader.cpp
are simply initial load attributes for the page.

however, that being said... this appears to be broken!!  neither the GIF
nor the CSS for www.mozilla.org get downloaded upon shift-reload.  but, i
think this is a separate bug that probably belongs to the docshell.

rpotts: am i correct in blaming docshell for not handling shift-reload correctly?

Comment 11

[David Hyatt]

Ah, ok.  XBL is set up correctly then.

Comment 12

[Bradley Baetz]

I thought I added this to the bug (maybe I only mentioned it to Ben on IRC): see
bug #46599, which was an RTM workaround for bug 29370, which is scheduled for
0.9. That bug mentions problems which are "timing related". No idea if they
still exist in the new cache, but this bug is probably a dupe of 29370.

images are probably a separate issue: see bug 73450. pavlov muttered the magic
"loadattributes" word in that.

Comment 13

[Pierre Saslawsky]

r=pierre

Comment 14

[Ben Goodger (use ben at mozilla dot org for email)]

Hm, I swear I put a comment in here before, but maybe bugzilla lost it. 

This is not just for CSS files, but for JS files loaded into HTML documents via 
the HTMLContentSink too. Look in ProcessSCRIPTTag in nsHTMLContentSink and 
you'll see js files being loaded with FORCE_RELOAD. nsXMLContentSink does not 
pass a final parameter, so the default of LOAD_NORMAL is used. So it seems that 
this patch needs to be applied in two places. 

Comment 15

[Chris Waterson]

sr=waterson. What's the bug # about inheriting the load attributes from parent?
Might want to refer to that from here...

Comment 16

[Darin Fisher]

fix checked in

Comment 17

[Biswapesh Chatterjee]

Probably a bit late, but perhaps the reason for this was bug #29370 ? See also
bug #17309 for related discussion.

- Biswa.

Comment 18

[Tom Everingham]

verified 7/10 builds - all platforms