We need to expose this to applications so they can notify the user in some ways when content from untrusted sources (e.g. from http) has been loaded from a document coming through a trusted connection (https).
Created attachment 178630 [details] Patch proposal + Unit tests Here comes a patch proposal. Asking for review
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Created attachment 178754 [details] Patch proposal + Unit tests Second patch after getting some feedback from Carlos in the hackfest
Comment on attachment 178754 [details] Patch proposal + Unit tests 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.
Thanks for the review Martin. Will address those issues right before landing. (In reply to comment #4) > (From update of attachment 178754 [details]) > 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. Yes, Carlos is fine with that too. > > 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" Ok. > > 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. Oops! Sorry. > > 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? Ok. > > Source/WebKit2/UIProcess/API/gtk/tests/TestSSL.cpp:157 > > + gsize length; > > + > > + g_file_get_contents(pathToFile.get(), &contents, &length, 0); > > Same here. Ok. > > 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. I think there's no need to change anything in this regard after the clarifications during the hackfest, so I'll leave it as it is now.
Committed r137450: <http://trac.webkit.org/changeset/137450>