[webkit-reviews] review granted: [Bug 46968] [Qt] QWebPage MIME type handling inconsistency with other web browsers : [Attachment 89123] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Apr 13 01:01:23 PDT 2011
Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Luiz Agostini
<luiz at webkit.org>'s request for review:
Bug 46968: [Qt] QWebPage MIME type handling inconsistency with other web
browsers
https://bugs.webkit.org/show_bug.cgi?id=46968
Attachment 89123: patch
https://bugs.webkit.org/attachment.cgi?id=89123&action=review
------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=89123&action=review
> Source/WebCore/platform/network/MIMESniffing.cpp:178
> + MAGIC_NUMBERS_MASKED("<A", "\xFF\xDF", "text/html", SkipWhiteSpace |
TrailingSpaceOrBracket), // <A
Why the // <A comment?
> Source/WebCore/platform/network/MIMESniffing.cpp:246
> + for (size_t i = 0; i < count; ++i)
> + if ((*data32++ & *mask32++) != *pattern32++)
> + return false;
Needs braces
> Source/WebCore/platform/network/MIMESniffing.cpp:250
> + const char* p = reinterpret_cast<const char*>(pattern32);
> + const char* m = reinterpret_cast<const char*>(mask32);
> + const char* d = reinterpret_cast<const char*>(data32);
Generally we should avoid variable names as these
> Source/WebCore/platform/network/MIMESniffing.cpp:257
> + for (size_t i = 0; i < count; ++i)
> + if ((*d++ & *m++) != *p++)
> + return false;
> +
Needs braces
> Source/WebCore/platform/network/MIMESniffing.cpp:388
> +const char RSSUrl[] = "http://purl.org/rss/1.0";
> +const char RDFUrl[] = "http://www.w3.org/1999/02/22-rdf-syntax-ns#";
rssUrl, rdfUrl
> Source/WebCore/platform/network/MIMESniffing.h:35
> +
> +
one newline too much
> Source/WebKit/qt/WebCoreSupport/FrameNetworkingContextQt.h:32
> + FrameNetworkingContextQt(Frame*, QObject* originatingObject, bool
MIMESniffingEnabled, QNetworkAccessManager*);
mimeTypeSniffingEnabled
> Source/WebKit/qt/WebCoreSupport/FrameNetworkingContextQt.h:40
> + bool m_MIMESniffingEnabled;
m_mimeTypeSniffingEnabled or mimeSniffingEnabled
> Source/WebKit/qt/tests/MIMESniffing/TestData.h:25
> + const char* advetisedType;
missing r!
> Source/WebKit/qt/tests/MIMESniffing/TestData.h:984
> +#endif // TESTDATA_H
TestData_h right?
> Source/WebKit/qt/tests/MIMESniffing/tst_MIMESniffing.cpp:54
> + for (int i = 0; i < testListSize; ++i) {
> +
> +
> + QFile file(testList[i].file);
two not needed newlines there
> Source/WebKit2/WebProcess/WebCoreSupport/qt/WebFrameNetworkingContext.cpp:54
> +bool WebFrameNetworkingContext::MIMESniffingEnabled() const
> +{
> + return m_MIMESniffingEnabled;
> +}
a method name does not start with capital :-) mimeTypeSniffingEnabled
> Source/WebKit2/WebProcess/WebCoreSupport/qt/WebFrameNetworkingContext.h:39
> + bool m_MIMESniffingEnabled;
Same here
More information about the webkit-reviews
mailing list