Bug 54343 - Web Inspector: audits should not warn about gzip compression for 304s
Summary: Web Inspector: audits should not warn about gzip compression for 304s
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexander Pavlov (apavlov)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-12 13:51 PST by Alexander Romanovich
Modified: 2011-02-21 10:02 PST (History)
13 users (show)

See Also:


Attachments
[PATCH] Suggested fix (2.26 KB, patch)
2011-02-14 08:38 PST, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
[PATCH] Comments addressed (2.22 KB, patch)
2011-02-21 07:10 PST, Alexander Pavlov (apavlov)
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Romanovich 2011-02-12 13:51:04 PST
It appears the audit tool is generating a red "Enable gzip compression" warning even if the resource in question was a "304 Not Modified" response with 0 content length. The gzip compression recommendation should be ignored in this case.
Comment 1 Alexey Proskuryakov 2011-02-12 23:27:07 PST
Maybe that's the reason for bug 33220?
Comment 2 Alexander Romanovich 2011-02-14 08:29:00 PST
Based on the patch for 33220, it appears not to be the reason. Which makes sense I guess. Audits are only run upon explicit user request, right?
Comment 3 Alexander Pavlov (apavlov) 2011-02-14 08:38:54 PST
Created attachment 82319 [details]
[PATCH] Suggested fix
Comment 4 Alexander Pavlov (apavlov) 2011-02-14 08:39:27 PST
(In reply to comment #2)
> Based on the patch for 33220, it appears not to be the reason. Which makes sense I guess. Audits are only run upon explicit user request, right?

Correct, it's a different thing.
Comment 5 Yury Semikhatsky 2011-02-21 05:19:12 PST
Comment on attachment 82319 [details]
[PATCH] Suggested fix

View in context: https://bugs.webkit.org/attachment.cgi?id=82319&action=review

> Source/WebCore/inspector/front-end/AuditRules.js:110
> +        var encodings = encodingHeader.split(/\s*,\s*/);

Consider replacing this code with a grep search.

> Source/WebCore/inspector/front-end/AuditRules.js:121
> +        return WebInspector.Resource.Type.isTextType(resource.type) && resource.domain && resource.statusCode !== 304 && resource.resourceSize !== undefined && resource.resourceSize > 150;

If 304s have 0 length content then we don't need to check for statusCode here. If resourceSize contains size of the whole response, we need to use another field which provides content length as only the content can be compressed.
Comment 6 Alexander Pavlov (apavlov) 2011-02-21 07:10:40 PST
Created attachment 83161 [details]
[PATCH] Comments addressed
Comment 7 Alexander Pavlov (apavlov) 2011-02-21 09:36:46 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       Source/WebCore/ChangeLog
        M       Source/WebCore/inspector/front-end/AuditRules.js
Committed r79229
Comment 8 WebKit Review Bot 2011-02-21 10:02:13 PST
http://trac.webkit.org/changeset/79229 might have broken Qt Linux Release
The following tests are not passing:
media/controls-without-preload.html