Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed #17552 -- Removed a hack for IE6 and earlier. #2781

Closed
wants to merge 1 commit into from
Closed

Fixed #17552 -- Removed a hack for IE6 and earlier. #2781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 1 addition & 7 deletions django/middleware/gzip.py
View file
Open in desktop
Expand Up @@ -17,17 +17,11 @@ def process_response(self, request, response):
if not response.streaming and len(response.content) < 200:
return response

patch_vary_headers(response, ('Accept-Encoding',))

# Avoid gzipping if we've already got a content-encoding.
if response.has_header('Content-Encoding'):
return response

# MSIE have issues with gzipped response of various content types.
if "msie" in request.META.get('HTTP_USER_AGENT', '').lower():
ctype = response.get('Content-Type', '').lower()
if not ctype.startswith("text/") or "javascript" in ctype:
return response
patch_vary_headers(response, ('Accept-Encoding',))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why it's okay to move patch_vary_headers()? (I know nothing about it and am just reading about it now.)

Also, remove (I think) the failing test.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving patch_vary_headers is a small performance optimization.

If this method returns before testing for the Accept-Encoding header, the response doesn't depend on Accept-Encoding, so it needs not be added to the Vary header. That's why it makes sense to put patch_vary_headers just before testing for Accept-Encoding.

(In fact, if the response already has a Content-Encoding header, a proper implementation willl have tested the Accept-Encoding header and called patch_vary_headers, so this doesn't change caching behavior in practice.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also removed the failing test (well done, pull request testing!)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I buy it. How about a more descriptive commit message (at least mention GZipMiddleware)?


ae = request.META.get('HTTP_ACCEPT_ENCODING', '')
if not re_accepts_gzip.search(ae):
Expand Down
5 changes: 0 additions & 5 deletions docs/ref/middleware.txt
View file
Open in desktop
Expand Up @@ -108,11 +108,6 @@ It will NOT compress content if any of the following are true:
* The request (the browser) hasn't sent an ``Accept-Encoding`` header
containing ``gzip``.

* The request is from Internet Explorer and the ``Content-Type`` header
contains ``javascript`` or starts with anything other than ``text/``.
We do this to avoid a bug in early versions of IE that caused decompression
not to be performed on certain content types.

You can apply GZip compression to individual views using the
:func:`~django.views.decorators.gzip.gzip_page()` decorator.

Expand Down
5 changes: 5 additions & 0 deletions docs/releases/1.8.txt
View file
Open in desktop
Expand Up @@ -273,6 +273,11 @@ Miscellaneous
* Database connections are considered equal only if they're the same object.
They aren't hashable any more.

* :class:`~django.middleware.gzip.GZipMiddleware` used to disable compression
for some content types when the request is from Internet Explorer, in order
to work around a bug in IE6 and earlier. This behavior could affect
performance on IE7 and later. It was removed.

* ``URLField.to_python`` no longer adds a trailing slash to pathless URLs.

* ``django.contrib.gis`` dropped support for GEOS 3.1 and GDAL 1.6.
Expand Down
10 changes: 0 additions & 10 deletions tests/middleware/tests.py
View file
Open in desktop
Expand Up @@ -613,16 +613,6 @@ def test_no_compress_compressed_response(self):
self.assertEqual(r.content, self.compressible_string)
self.assertEqual(r.get('Content-Encoding'), 'deflate')

def test_no_compress_ie_js_requests(self):
"""
Tests that compression isn't performed on JavaScript requests from Internet Explorer.
"""
self.req.META['HTTP_USER_AGENT'] = 'Mozilla/4.0 (compatible; MSIE 5.00; Windows 98)'
self.resp['Content-Type'] = 'application/javascript; charset=UTF-8'
r = GZipMiddleware().process_response(self.req, self.resp)
self.assertEqual(r.content, self.compressible_string)
self.assertEqual(r.get('Content-Encoding'), None)

def test_no_compress_uncompressible_response(self):
"""
Tests that compression isn't performed on responses with uncompressible content.
Expand Down