Title: [230818] trunk
Revision
230818
Author
cdu...@apple.com
Date
2018-04-19 13:57:52 -0700 (Thu, 19 Apr 2018)

Log Message

REGRESSION (r229133): decidePolicyForNavigationAction not called for loading an HTML string
https://bugs.webkit.org/show_bug.cgi?id=184209
<rdar://problem/39145306>

Reviewed by Ryosuke Niwa.

Source/WebCore:

In r229133, we stopped doing navigation policy checks for about:blank because about:blank
loads need to happen synchronously for Web-compatibility. However, this regressed loading
an HTML string in a WebView because in such cases, the URL is also about:blank with
substitute data.

In this patch, we take a more conservative approach and restore policy checking for
'about:blank' but using synchronous IPC.

* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNavigationPolicy):

Tools:

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm:
(TEST):

LayoutTests:

Rebaseline existing layout tests. Their output is back to what it was before r229133.

* fast/loader/iframe-src-invalid-url-expected.txt:
* fast/loader/policy-delegate-action-hit-test-zoomed-expected.txt:
* loader/navigation-policy/should-open-external-urls/subframe-click-target-self-expected.txt:
* loader/navigation-policy/should-open-external-urls/subframe-click-target-top-expected.txt:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (230817 => 230818)


--- trunk/LayoutTests/ChangeLog	2018-04-19 20:37:31 UTC (rev 230817)
+++ trunk/LayoutTests/ChangeLog	2018-04-19 20:57:52 UTC (rev 230818)
@@ -1,3 +1,18 @@
+2018-04-19  Chris Dumez  <cdu...@apple.com>
+
+        REGRESSION (r229133): decidePolicyForNavigationAction not called for loading an HTML string
+        https://bugs.webkit.org/show_bug.cgi?id=184209
+        <rdar://problem/39145306>
+
+        Reviewed by Ryosuke Niwa.
+
+        Rebaseline existing layout tests. Their output is back to what it was before r229133.
+
+        * fast/loader/iframe-src-invalid-url-expected.txt:
+        * fast/loader/policy-delegate-action-hit-test-zoomed-expected.txt:
+        * loader/navigation-policy/should-open-external-urls/subframe-click-target-self-expected.txt:
+        * loader/navigation-policy/should-open-external-urls/subframe-click-target-top-expected.txt:
+
 2018-04-19  Chris Nardi  <cna...@chromium.org>
 
         Support calc() in webkit-gradient and cross-fade

Modified: trunk/LayoutTests/fast/loader/iframe-src-invalid-url-expected.txt (230817 => 230818)


--- trunk/LayoutTests/fast/loader/iframe-src-invalid-url-expected.txt	2018-04-19 20:37:31 UTC (rev 230817)
+++ trunk/LayoutTests/fast/loader/iframe-src-invalid-url-expected.txt	2018-04-19 20:57:52 UTC (rev 230818)
@@ -1,5 +1,7 @@
  - decidePolicyForNavigationAction 
 <NSURLRequest URL data:text/html,    <iframe id='iframe' src=''></iframe>, main document URL iframe-src-invalid-url.html, http method GET> is main frame - no should open URLs externally - no
+ - decidePolicyForNavigationAction 
+<NSURLRequest URL about:blank, main document URL iframe-src-invalid-url.html, http method GET> is main frame - no should open URLs externally - no
 Test passes if the second iframe navigates to about:blank.
 
 

Modified: trunk/LayoutTests/fast/loader/policy-delegate-action-hit-test-zoomed-expected.txt (230817 => 230818)


--- trunk/LayoutTests/fast/loader/policy-delegate-action-hit-test-zoomed-expected.txt	2018-04-19 20:37:31 UTC (rev 230817)
+++ trunk/LayoutTests/fast/loader/policy-delegate-action-hit-test-zoomed-expected.txt	2018-04-19 20:57:52 UTC (rev 230818)
@@ -1,3 +1,4 @@
+Policy delegate: attempt to load about:blank with navigation type 'link clicked' originating from #text > A > BODY > HTML > #document
 This is a test for rdar://problem/6755137 Action dictionary for policy decision is missing keys when full-page zoom is used.
 
 Link

Modified: trunk/LayoutTests/loader/navigation-policy/should-open-external-urls/subframe-click-target-self-expected.txt (230817 => 230818)


--- trunk/LayoutTests/loader/navigation-policy/should-open-external-urls/subframe-click-target-self-expected.txt	2018-04-19 20:37:31 UTC (rev 230817)
+++ trunk/LayoutTests/loader/navigation-policy/should-open-external-urls/subframe-click-target-self-expected.txt	2018-04-19 20:57:52 UTC (rev 230818)
@@ -1,4 +1,6 @@
  - decidePolicyForNavigationAction 
+<NSURLRequest URL about:blank, main document URL subframe-click-target-self.html, http method GET> is main frame - no should open URLs externally - no
+ - decidePolicyForNavigationAction 
 <NSURLRequest URL resources/iframe-click-notify-done-target-self.html, main document URL subframe-click-target-self.html, http method GET> is main frame - no should open URLs externally - no
  - decidePolicyForNavigationAction 
 <NSURLRequest URL resources/notify-done.html, main document URL subframe-click-target-self.html, http method GET> is main frame - no should open URLs externally - yes

Modified: trunk/LayoutTests/loader/navigation-policy/should-open-external-urls/subframe-click-target-top-expected.txt (230817 => 230818)


--- trunk/LayoutTests/loader/navigation-policy/should-open-external-urls/subframe-click-target-top-expected.txt	2018-04-19 20:37:31 UTC (rev 230817)
+++ trunk/LayoutTests/loader/navigation-policy/should-open-external-urls/subframe-click-target-top-expected.txt	2018-04-19 20:57:52 UTC (rev 230818)
@@ -1,4 +1,6 @@
  - decidePolicyForNavigationAction 
+<NSURLRequest URL about:blank, main document URL subframe-click-target-top.html, http method GET> is main frame - no should open URLs externally - no
+ - decidePolicyForNavigationAction 
 <NSURLRequest URL resources/iframe-click-notify-done-target-top.html, main document URL subframe-click-target-top.html, http method GET> is main frame - no should open URLs externally - no
  - decidePolicyForNavigationAction 
 <NSURLRequest URL resources/notify-done.html, main document URL resources/notify-done.html, http method GET> is main frame - yes should open URLs externally - yes

Modified: trunk/Source/WebCore/ChangeLog (230817 => 230818)


--- trunk/Source/WebCore/ChangeLog	2018-04-19 20:37:31 UTC (rev 230817)
+++ trunk/Source/WebCore/ChangeLog	2018-04-19 20:57:52 UTC (rev 230818)
@@ -1,3 +1,22 @@
+2018-04-19  Chris Dumez  <cdu...@apple.com>
+
+        REGRESSION (r229133): decidePolicyForNavigationAction not called for loading an HTML string
+        https://bugs.webkit.org/show_bug.cgi?id=184209
+        <rdar://problem/39145306>
+
+        Reviewed by Ryosuke Niwa.
+
+        In r229133, we stopped doing navigation policy checks for about:blank because about:blank
+        loads need to happen synchronously for Web-compatibility. However, this regressed loading
+        an HTML string in a WebView because in such cases, the URL is also about:blank with
+        substitute data.
+
+        In this patch, we take a more conservative approach and restore policy checking for
+        'about:blank' but using synchronous IPC.
+
+        * loader/PolicyChecker.cpp:
+        (WebCore::PolicyChecker::checkNavigationPolicy):
+
 2018-04-19  Chris Nardi  <cna...@chromium.org>
 
         Support calc() in webkit-gradient and cross-fade

Modified: trunk/Source/WebCore/loader/PolicyChecker.cpp (230817 => 230818)


--- trunk/Source/WebCore/loader/PolicyChecker.cpp	2018-04-19 20:37:31 UTC (rev 230817)
+++ trunk/Source/WebCore/loader/PolicyChecker.cpp	2018-04-19 20:57:52 UTC (rev 230818)
@@ -123,8 +123,8 @@
 
     loader->setLastCheckedRequest(ResourceRequest(request));
 
-    if (request.url() == blankURL())
-        return function(WTFMove(request), formState, ShouldContinue::Yes);
+    if (request.url().isBlankURL())
+        policyDecisionMode = PolicyDecisionMode::Synchronous;
 
 #if USE(QUICK_LOOK)
     // Always allow QuickLook-generated URLs based on the protocol scheme.

Modified: trunk/Tools/ChangeLog (230817 => 230818)


--- trunk/Tools/ChangeLog	2018-04-19 20:37:31 UTC (rev 230817)
+++ trunk/Tools/ChangeLog	2018-04-19 20:57:52 UTC (rev 230818)
@@ -1,3 +1,16 @@
+2018-04-19  Chris Dumez  <cdu...@apple.com>
+
+        REGRESSION (r229133): decidePolicyForNavigationAction not called for loading an HTML string
+        https://bugs.webkit.org/show_bug.cgi?id=184209
+        <rdar://problem/39145306>
+
+        Reviewed by Ryosuke Niwa.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm:
+        (TEST):
+
 2018-04-18  Ross Kirsling  <ross.kirsl...@sony.com>
 
         [WinCairo][EWS] Build bot should clean user temp directory each time.

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm (230817 => 230818)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm	2018-04-19 20:37:31 UTC (rev 230817)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm	2018-04-19 20:57:52 UTC (rev 230818)
@@ -285,6 +285,22 @@
     action = ""
 }
 
+TEST(WebKit, DecidePolicyForNavigationActionForLoadHTMLString)
+{
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+
+    auto window = adoptNS([[NSWindow alloc] initWithContentRect:[webView frame] styleMask:NSWindowStyleMaskBorderless backing:NSBackingStoreBuffered defer:YES]);
+    [[window contentView] addSubview:webView.get()];
+
+    auto controller = adoptNS([[DecidePolicyForNavigationActionController alloc] init]);
+    [webView setNavigationDelegate:controller.get()];
+    [webView setUIDelegate:controller.get()];
+
+    decidedPolicy = false;
+    [webView loadHTMLString:@"TEST" baseURL:[NSURL URLWithString:@"about:blank"]];
+    TestWebKitAPI::Util::run(&decidedPolicy);
+}
+
 TEST(WebKit, DecidePolicyForNavigationActionForTargetedWindowOpen)
 {
     auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to