[webkit-reviews] review denied: [Bug 58378] Add WebCore::Setting to block displaying and/or running insecure content on secure pages : [Attachment 150427] Plumbing #3

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jul 3 08:22:15 PDT 2012


Martin Robinson <mrobinson at webkit.org> has denied  review:
Bug 58378: Add WebCore::Setting to block displaying and/or running insecure
content on secure pages
https://bugs.webkit.org/show_bug.cgi?id=58378

Attachment 150427: Plumbing #3
https://bugs.webkit.org/attachment.cgi?id=150427&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=150427&action=review


If you'd like your patch you be reviewed, when posting it you should change the
"r" field to say "r?"

I'm not sure why you're adding hooks for this into DumpRenderTree, but we
talked about that yesterday. Perhaps it would be better to add unit tests for
GTK+ in Source/WebKit/gtk/tests.

The final thing is that maybe this should be only a single setting instead of
two. Something like allow-display-and-running-of-insecure-content. I'm not sure
about that.

> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:1020
> +    * Boolean property to control whether insecure (non-https) images /
frames 
> +    * can be loaded from https pages.

Nit: Should be HTTPS not https. It's probably not necessary to say this a
boolean property, you could simply write.

Whether is it possible for pages loaded via HTTPS to display subresources, such
as images and frames, from non-HTTPS URLs.

> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:1026
> +							    _("Controls load of
non-https display resources from https pages"),

How about:

Whether non-HTTPS resources can display on HTTPS pages.

> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:1034
> +    * Boolean property to control whether insecure (non-https) script / CSS
/ 
> +    * plug-ins can be loaded from https pages.

Same here you could write something like:

Whether is it possible for pages loaded via HTTPS to run subresources, such as
CSS, scripts and plugin, from non-HTTPS URLs.

> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:1040
> +							    _("Controls load of
non-https script / plug-in resources from https pages."),

Whether non-HTTPS resources can run on HTTPS pages.


More information about the webkit-reviews mailing list