Since two platforms now share this same code in their WebKit layers, it seems okay that this was moved into some shared location. But I have a few gripes:
HTTPParsers is about parsing HTTP, not implementing policy. A better place is probably ResourceResponse itself. But as written the method implements a client policy. This reeks of breaking the layering between WebCore and WebKit. I would feel a lot better about it if the method was: String ResourceResponseBase::contentDispositionType() const; ...and then the clients made their own decision based on that. Safari actually does this, for example. I'll add these comments to the bug. ~Brady On Mar 29, 2010, at 4:58 PM, e...@webkit.org wrote: > Revision > 56750 > Author > e...@webkit.org > Date > 2010-03-29 16:58:03 -0700 (Mon, 29 Mar 2010) > Modified: trunk/WebCore/platform/network/HTTPParsers.cpp (56749 => 56750) > > --- trunk/WebCore/platform/network/HTTPParsers.cpp 2010-03-29 23:38:40 UTC > (rev 56749) > +++ trunk/WebCore/platform/network/HTTPParsers.cpp 2010-03-29 23:58:03 UTC > (rev 56750) > @@ -2,6 +2,7 @@ > > +bool shouldTreatAsAttachment(const ResourceResponseBase& response) > +{ > + const String& contentDisposition = > response.httpHeaderField("Content-Disposition"); > + > + if (contentDisposition.isEmpty()) > + return false; > + > + // Some broken sites just send > + // Content-Disposition: ; filename="file" > + // screen those out here. > + if (contentDisposition.startsWith(";")) > + return false; > + > + if (contentDisposition.startsWith("inline", false)) > + return false; > + > + // Some broken sites just send > + // Content-Disposition: filename="file" > + // without a disposition token... screen those out. > + if (contentDisposition.startsWith("filename", false)) > + return false; > + > + // Also in use is Content-Disposition: name="file" > + if (contentDisposition.startsWith("name", false)) > + return false; > + > + // We have a content-disposition of "attachment" or unknown. > + // RFC 2183, section 2.8 says that an unknown disposition > + // value should be treated as "attachment" > + return true; > +} > + > bool parseHTTPRefresh(const String& refresh, bool fromHttpEquivMeta, double& > delay, String& url) > { > int len = refresh.length(); > _______________________________________________ > webkit-changes mailing list > webkit-chan...@lists.webkit.org > http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev