Bug 54343

Summary: Web Inspector: audits should not warn about gzip compression for 304s
Product: WebKit Reporter: Alexander Romanovich <alex>
Component: Web Inspector (Deprecated)Assignee: Alexander Pavlov (apavlov) <apavlov>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, apavlov, bweinstein, eric, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Suggested fix
none
[PATCH] Comments addressed yurys: review+

Alexander Romanovich
Reported 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.
Attachments
[PATCH] Suggested fix (2.26 KB, patch)
2011-02-14 08:38 PST, Alexander Pavlov (apavlov)
no flags
[PATCH] Comments addressed (2.22 KB, patch)
2011-02-21 07:10 PST, Alexander Pavlov (apavlov)
yurys: review+
Alexey Proskuryakov
Comment 1 2011-02-12 23:27:07 PST
Maybe that's the reason for bug 33220?
Alexander Romanovich
Comment 2 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?
Alexander Pavlov (apavlov)
Comment 3 2011-02-14 08:38:54 PST
Created attachment 82319 [details] [PATCH] Suggested fix
Alexander Pavlov (apavlov)
Comment 4 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.
Yury Semikhatsky
Comment 5 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.
Alexander Pavlov (apavlov)
Comment 6 2011-02-21 07:10:40 PST
Created attachment 83161 [details] [PATCH] Comments addressed
Alexander Pavlov (apavlov)
Comment 7 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
WebKit Review Bot
Comment 8 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
Note You need to log in before you can comment on or make changes to this bug.