[webkit-reviews] review granted: [Bug 104578] [GTK][WK2] New API to detect display/execution of insecure content : [Attachment 178754] Patch proposal + Unit tests
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Dec 12 02:37:59 PST 2012
Martin Robinson <mrobinson at webkit.org> has granted Mario Sanchez Prada
<mario at webkit.org>'s request for review:
Bug 104578: [GTK][WK2] New API to detect display/execution of insecure content
https://bugs.webkit.org/show_bug.cgi?id=104578
Attachment 178754: Patch proposal + Unit tests
https://bugs.webkit.org/attachment.cgi?id=178754&action=review
------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=178754&action=review
Looks good, as long as cgarcia is okay with the fact that
WEBKIT_INSECURE_CONTENT_RUN is not called WEBKIT_INSECURE_CONTENT_EVENT_RUN and
WEBKIT_INSECURE_CONTENT_DISPLAYED is not called
WEBKIT_INSECURE_CONTENT_EVENT_DISPLAYED. I don't think they are too long, but
it's not a huge deal.
> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1245
> + * This signal is emitted when insecure content has been detected
> + * in a page loaded through a secure connection. This typically
> + * means that an external resource from an unstrusted source
> + * (e.g. non HTTPS) has been run or displayed.
> + *
You might want to try to more clearly describe the situation as "mixing of
HTTPS and non-HTTPS content"
> Source/WebKit2/UIProcess/API/gtk/tests/TestSSL.cpp:92
> + static void insecure_content_detected_callback(WebKitWebView* webView,
WebKitInsecureContentEvent event, InsecureContentTest* test)
Please use WebKit naming conventions here.
> Source/WebKit2/UIProcess/API/gtk/tests/TestSSL.cpp:148
> + char* contents;
> + gsize length;
> +
> + g_file_get_contents(pathToFile.get(), &contents, &length, 0);
Okay. Super nit, but wouldn't it make more sense to "glue" the declarations
with the statement that uses them?
> Source/WebKit2/UIProcess/API/gtk/tests/TestSSL.cpp:157
> + gsize length;
> +
> + g_file_get_contents(pathToFile.get(), &contents, &length, 0);
Same here.
> Tools/Scripts/webkitpy/style/checker.py:184
> + "-readability/enum_casing",
> "-whitespace/parens"]),
> +
> + ([# The GTK+ API use upper case, underscore separated, words in
> + # certain types of enums (e.g. signals, properties).
> + "Source/WebKit2/UIProcess/API/gtk"],
> + ["-readability/enum_casing"]),
I think maybe this is a bit too broad. We want to detect enum style violations
in these files too. Perhaps just turn webkitWebViewInsecureContentDetected into
webkitWebViewInsecureContentRun and webkitWebViewInsecureContentDisplayed.
More information about the webkit-reviews
mailing list