[webkit-reviews] review denied: [Bug 174992] Layout tests with 'https' suffix should be run over HTTPS : [Attachment 316856] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 1 08:54:15 PDT 2017


Darin Adler <darin at apple.com> has denied youenn fablet <youennf at gmail.com>'s
request for review:
Bug 174992: Layout tests with 'https' suffix should be run over HTTPS
https://bugs.webkit.org/show_bug.cgi?id=174992

Attachment 316856: Patch

https://bugs.webkit.org/attachment.cgi?id=316856&action=review




--- Comment #6 from Darin Adler <darin at apple.com> ---
Comment on attachment 316856
  --> https://bugs.webkit.org/attachment.cgi?id=316856
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=316856&action=review

Concept seems fine. But not everything needed is here.

> Source/WebCore/testing/Internals.cpp:3930
> +
> +    auto* frame = document->frame();
> +    if (!frame)
> +	   return;
> +
> +    frame->settings().setAllowDisplayOfInsecureContent(true);

Should just be:

    document->settings().setAllowDisplayOfInsecureContent(true);

No need to involve the Frame.

Do we have something that changes this setting back so it won’t affect
subsequent tests? I don’t see anything new in this patch, but maybe it already
exists?

>
LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests
/iframe-upgrade.https.html:44
> +    if (window.internals)
> +	   internals.setStrictMixedContentMode(false);

This looks wrong. It’s not calling the new function you added. Maybe based on
an earlier try?


More information about the webkit-reviews mailing list