REGRESSION (r229133): decidePolicyForNavigationAction not called for loading an HTML...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Apr 2018 20:57:52 +0000 (20:57 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Apr 2018 20:57:52 +0000 (20:57 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230818 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/loader/iframe-src-invalid-url-expected.txt
LayoutTests/fast/loader/policy-delegate-action-hit-test-zoomed-expected.txt
LayoutTests/loader/navigation-policy/should-open-external-urls/subframe-click-target-self-expected.txt
LayoutTests/loader/navigation-policy/should-open-external-urls/subframe-click-target-top-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/loader/PolicyChecker.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm

index 1beeafbc7bfc77f5f26aaac0e9298c8d69dfc4b3..e6b97d94c814c1e8569a5cd2c779085d960a51c8 100644 (file)
@@ -1,3 +1,18 @@
+2018-04-19  Chris Dumez  <cdumez@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  <cnardi@chromium.org>
 
         Support calc() in webkit-gradient and cross-fade
index 6a6d071b7d60b6f0fa97f33fcc0c8a2472fe970c..d99ecef4ee26f7630c4572dbe1b4a1a512d7f754 100644 (file)
@@ -1,5 +1,7 @@
  - decidePolicyForNavigationAction 
 <NSURLRequest URL data:text/html,    <iframe id='iframe' src='<p>FAILURE</p>'></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.
 
 
index cf93a2aabba08e95bc180a1e1f1684542349f621..478d311217e8075a7f0e82047c72e97c75ef7561 100644 (file)
@@ -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
index eaab61dc8f04b6ee3b5c069c0a01e090d1fd531d..c30fe8b7165c3a250e9cede73c72e6595612a42e 100644 (file)
@@ -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
index f0990cb6d38f5e42a1ea1abb7abde5b443220bb5..9b5d3f434edb147aaf5719cefa60750fd4396f08 100644 (file)
@@ -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
index b7151eff97e17e27e9e184d7af87af9683c4dec4..ae89cf2f72933563291d23b174210e8818bcb1d2 100644 (file)
@@ -1,3 +1,22 @@
+2018-04-19  Chris Dumez  <cdumez@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  <cnardi@chromium.org>
 
         Support calc() in webkit-gradient and cross-fade
index 82d6f697861e4bf396c23f0589d4083f4b72e853..81318a7aa00ad342c55027903018f0cfb2e486b2 100644 (file)
@@ -123,8 +123,8 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
 
     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.
index a5f70d73c1546f43165c15d10102fe7d2e25c11e..5e1c2fb98066e8874e5c58e4414dc50e6bebbcf1 100644 (file)
@@ -1,3 +1,16 @@
+2018-04-19  Chris Dumez  <cdumez@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.kirsling@sony.com>
 
         [WinCairo][EWS] Build bot should clean user temp directory each time.
index 8b15e31f47d9da70817e3679e1e05869f9d483dd..31116aac213b579f7b6ffbab218708d60563547d 100644 (file)
@@ -285,6 +285,22 @@ TEST(WebKit, DecidePolicyForNavigationActionForTargetedHyperlink)
     action = nullptr;
 }
 
+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)]);