Whilst porting some of our HTTP header handling to the kernel patch, I ran into this little gem:

http://api.drupal.org/api/drupal/includes!bootstrap.inc/function/drupal_page_header/7

Have a look at the huge paragraph there. Now read it again to try and understand what it's doing. It may take several tries. :-)

Now, have a look at the Firefox bug that it links to: https://bugzilla.mozilla.org/show_bug.cgi?id=269303

That was marked as a duplicate of another issue in 2009: https://bugzilla.mozilla.org/show_bug.cgi?id=510359

That issue was also resolved in 2009. So it will have been fixed for ~4 years by the time Drupal ships.

OK, but does that mean we don't need to support that browser version?

According to Firefox's release history, the bug would have been fixed before Firefox 3.6 shipped, and was likely also backported to Firefox 3.5. That means the only extant versions of Firefox with this bug should be 3.0 and earlier. Firefox 3.0's market share is, I think, in negative numbers at this point as Mozilla just announced that they have enabled auto-update for Firefox 3.6, so all Firefox 3.6 browsers will overnight turn into Firefox 13 in the not too distant future.

That is, browsers affected by that bug no longer exist in the wild.

We should therefore drop that hack entirely, and free up the use of ETags for something useful, like better cache control.

However, let us NOT do that until after the #1463656: Add a Drupal kernel; leverage HttpFoundation and HttpKernel lands, because I don't want yet another merge conflict. :-) It should be easy to strip out afterward.

Comments

chx’s picture

Do we have a "huge can of worms" tag :D ? REQUEST_TIME sucks as an ETag but then again, the "what's a good ETag" is an insanely complicated question that is not easily answered. Hashing the content is not bad... unless the hash itself is a performance hit...

Crell’s picture

Issue tags: +huge can of worms

To be honest, I don't fully comprehend how ETags work in the first place. That's one reason I'd rather not use them if we don't need to. Symfony does some futzing with the cache headers on its own to make them sane, so unless we know exactly what we're doing I'm inclined to get out of its way and let it figure things out.

Right now Drupal goes to great lengths to defeat HTTP caching. That's something we need to stop doing. This is just a part of that.

sun’s picture

http://drupalcode.org/project/drupal.git/commit/526401c4c was added in

#147310: Implement better cache headers for reverse proxies

As usual, someone should double-check whether the phpDoc is actually complete and correct, and covers all problems the change intended to fix.

mgifford’s picture

I came onto this when looking at a Varnish issue (one page had a different ETag with every load and wasn't caching, while others kept with the same etag no matter how many times we loaded the file).

If @chx & @crell don't understand this, I'm certain we need better docs. More than just:
https://en.wikipedia.org/wiki/HTTP_ETag

I'm assuming the docs here haven't been updated - http://api.drupal.org/api/drupal/includes!bootstrap.inc/function/drupal_...

@sun these are the phpDocs right?

Adding links to other references to ETag best practices:
http://realityloop.com/blog/2009/08/19/optimizing-page-load-times-using-...
http://wimleers.com/article/improving-drupals-page-loading-performance

mgifford’s picture

Issue summary: View changes

fix link

znerol’s picture

vijaycs85’s picture

as per https://www.drupal.org/node/2181523 we don't have drupal_page_header in D8. Is this still valid issue?

vijaycs85’s picture

Only place we have this in D8 is install_display_output. So this issue is 'Closed (Won't fix)' to me.

mgifford’s picture

Status: Active » Closed (won't fix)

I looked at the output too and there is no ETag in D8.