Bug 142387 - REGRESSION(r181134): [GTK] Test /webkit2/WebKitWebView/insecure-content is failing after r181134
Summary: REGRESSION(r181134): [GTK] Test /webkit2/WebKitWebView/insecure-content is fa...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords: Gtk, Regression
: 142663 (view as bug list)
Depends on:
Blocks: 140625
  Show dependency treegraph
 
Reported: 2015-03-06 00:08 PST by Carlos Garcia Campos
Modified: 2015-04-13 08:07 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.66 KB, patch)
2015-04-13 07:47 PDT, Sergio Villar Senin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2015-03-06 00:08:42 PST
Since r181134 we are now blocking mixed content by default (except for images if I understood the patch correctly, where we still show a warning in the console message, and the client is notified), which is indeed good news. The test is only failing for the run-insecure-content, not the display one because it uses an image. 
So, we should probably check now that the load failed in provisional load state. The thing is, since we don't expose the allowDisplayOfInsecureContent nor the allowRunningOfInsecureContent settings in the API, WEBKIT_INSECURE_CONTENT_RUN is now impossible to happen. Fortunately we didn't document any default policy in our API docs, so we can just change it, but I think we should document the new behaviour, explaining that running insecure content is always blocked, that WEBKIT_INSECURE_CONTENT_RUN is deprecated, so WebKitWebView::insecure-content-detected will only be called with WEBKIT_INSECURE_CONTENT_DISPLAYED and only for images, because all other contents will be blocked producing a load failure of the resource. 
Or we could expose the settings in our API, and keeping our current unit tests but enabling the settings (I would still keep the setting disabled by default in this case, even if it's not backwards compatible).
Comment 1 Michael Catanzaro 2015-03-06 06:52:34 PST
Bug #140392 adds API to expose those settings. Currently all browsers that block mixed content have UI to disable blocking on the current page. As much as I'd like to avoid that, because *most* sites will not break, we probably have to add it. So we will still need WEBKIT_INSECURE_CONTENT_RUN.

The change in default will be good, but only for 2.10.
Comment 2 Michael Catanzaro 2015-03-06 07:01:07 PST
Also the patch in that bug probably fixes the test case, but it changes cross-platform code to add WEBKIT_INSECURE_CONTENT_BLOCKED so we need an Apple reviewer.
Comment 3 Carlos Garcia Campos 2015-03-06 08:29:56 PST
(In reply to comment #1)
> Bug #140392 adds API to expose those settings. Currently all browsers that
> block mixed content have UI to disable blocking on the current page. As much
> as I'd like to avoid that, because *most* sites will not break, we probably
> have to add it. So we will still need WEBKIT_INSECURE_CONTENT_RUN.
> 
> The change in default will be good, but only for 2.10.

Yes, this doesn't affect any of the stable branches.
Comment 4 Carlos Garcia Campos 2015-03-13 01:48:13 PDT
*** Bug 142663 has been marked as a duplicate of this bug. ***
Comment 5 Michael Catanzaro 2015-03-13 07:09:06 PDT
Note that we can't do anything about this (besides skip the test) until we get approval from Apple on how to handle bug #142469.
Comment 6 Carlos Garcia Campos 2015-03-13 07:30:35 PDT
(In reply to comment #5)
> Note that we can't do anything about this (besides skip the test) until we
> get approval from Apple on how to handle bug #142469.

Since the test is only failing for the run-execute-content I prefer to add #if 0 to the affected part with a FIXME pointing to this bug instead of skipping the test.
Comment 7 Sergio Villar Senin 2015-04-13 07:47:35 PDT
Created attachment 250638 [details]
Patch

Let's update the expected result instead of indefinitely wait for some other bugs
Comment 8 Carlos Garcia Campos 2015-04-13 08:04:01 PDT
Comment on attachment 250638 [details]
Patch

Ok, we will update this test when the new api is added
Comment 9 Sergio Villar Senin 2015-04-13 08:07:27 PDT
Comment on attachment 250638 [details]
Patch

Clearing flags on attachment: 250638

Committed r182733: <http://trac.webkit.org/changeset/182733>
Comment 10 Sergio Villar Senin 2015-04-13 08:07:44 PDT
All reviewed patches have been landed.  Closing bug.