- Revision
- 229260
- Author
- [email protected]
- Date
- 2018-03-05 04:34:23 -0800 (Mon, 05 Mar 2018)
Log Message
Merge r229108 - html/browsers/browsing-the-web/navigating-across-documents/006.html fails with async policy delegates
https://bugs.webkit.org/show_bug.cgi?id=183168
<rdar://problem/37951341>
Reviewed by Alex Christensen.
Source/WebCore:
The test has an anchor element with both a 'click' event handler which submits a form
and an href attribute. When clicking the link, as per specification, things happen in
this order:
1. We fire the click event at the anchor, which will execute the event handler and submit the form.
Submitting the form *schedules* a navigation to 'click.html'.
2. We execute the anchor activation code which *navigates* to 'href.html'. The navigation to
'href' is supposed to cancel the pending navigation to 'click.html' and we should navigate
to 'href.html', which is what the test asserts.
The issue for us is that we do not cancel pending navigations until after the navigation
policy decision is made, when the provisional loads actually starts, in FrameLoader::provisionalLoadStarted().
Because the policy decision for the navigation can now be made asynchronously, the NavigationScheduler
timer can now fire while the decision is made and we'll submit the form, thus navigating to
'click.html'.
To address the issue, we now cancel any pending navigations in FrameLoader::loadWithDocumentLoader(),
*before* doing the policy check for the navigation.
Test: http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadWithDocumentLoader):
LayoutTests:
Add layout test coverage.
* TestExpectations:
* http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt: Added.
* http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html: Added.
* http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html: Added.
* http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html: Added.
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog (229259 => 229260)
--- releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog 2018-03-05 12:34:12 UTC (rev 229259)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog 2018-03-05 12:34:23 UTC (rev 229260)
@@ -1,3 +1,19 @@
+2018-02-28 Chris Dumez <[email protected]>
+
+ html/browsers/browsing-the-web/navigating-across-documents/006.html fails with async policy delegates
+ https://bugs.webkit.org/show_bug.cgi?id=183168
+ <rdar://problem/37951341>
+
+ Reviewed by Alex Christensen.
+
+ Add layout test coverage.
+
+ * TestExpectations:
+ * http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt: Added.
+ * http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html: Added.
+ * http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html: Added.
+ * http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html: Added.
+
2018-02-26 Commit Queue <[email protected]>
Unreviewed, rolling out r226745.
Modified: releases/WebKitGTK/webkit-2.20/LayoutTests/TestExpectations (229259 => 229260)
--- releases/WebKitGTK/webkit-2.20/LayoutTests/TestExpectations 2018-03-05 12:34:12 UTC (rev 229259)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/TestExpectations 2018-03-05 12:34:23 UTC (rev 229260)
@@ -296,6 +296,10 @@
# This is a resources folder.
http/tests/workers/service/other_resources [ Skip ]
+# These are resource files.
+http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html
+http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html
+
# We fail this reftest
webkit.org/b/179881 imported/w3c/web-platform-tests/encoding/eof-shift_jis.html [ ImageOnlyFailure ]
Added: releases/WebKitGTK/webkit-2.20/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt (0 => 229260)
--- releases/WebKitGTK/webkit-2.20/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt 2018-03-05 12:34:23 UTC (rev 229260)
@@ -0,0 +1,4 @@
+
+PASS Link with onclick form submit and href navigation
+
+Test
Added: releases/WebKitGTK/webkit-2.20/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html (0 => 229260)
--- releases/WebKitGTK/webkit-2.20/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html (rev 0)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html 2018-03-05 12:34:23 UTC (rev 229260)
@@ -0,0 +1,21 @@
+<!doctype html>
+<title>Link with onclick form submit and href navigation </title>
+<script src=""
+<script src=""
+<script>
+if (window.testRunner && testRunner.setShouldDecideNavigationPolicyAfterDelay)
+ testRunner.setShouldDecideNavigationPolicyAfterDelay(true);
+</script>
+<div id="log"></div>
+<iframe name="test"></iframe>
+<form target="test" action=""
+<a target="test" _onclick_="document.forms[0].submit()" href=""
+<script>
+var t = async_test();
+t.step(function() {document.links[0].click()});
+_onmessage_ = t.step_func(
+ function(e) {
+ assert_equals(e.data, "href");
+ t.done();
+ });
+</script>
Added: releases/WebKitGTK/webkit-2.20/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html (0 => 229260)
--- releases/WebKitGTK/webkit-2.20/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html (rev 0)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html 2018-03-05 12:34:23 UTC (rev 229260)
@@ -0,0 +1,4 @@
+<!doctype html>
+<script>
+parent.postMessage("click", "*");
+</script>
Added: releases/WebKitGTK/webkit-2.20/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html (0 => 229260)
--- releases/WebKitGTK/webkit-2.20/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html (rev 0)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html 2018-03-05 12:34:23 UTC (rev 229260)
@@ -0,0 +1,5 @@
+<!doctype html>
+<script>
+parent.postMessage("href", "*");
+</script>
+href
Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog (229259 => 229260)
--- releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog 2018-03-05 12:34:12 UTC (rev 229259)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog 2018-03-05 12:34:23 UTC (rev 229260)
@@ -1,3 +1,34 @@
+2018-02-28 Chris Dumez <[email protected]>
+
+ html/browsers/browsing-the-web/navigating-across-documents/006.html fails with async policy delegates
+ https://bugs.webkit.org/show_bug.cgi?id=183168
+ <rdar://problem/37951341>
+
+ Reviewed by Alex Christensen.
+
+ The test has an anchor element with both a 'click' event handler which submits a form
+ and an href attribute. When clicking the link, as per specification, things happen in
+ this order:
+ 1. We fire the click event at the anchor, which will execute the event handler and submit the form.
+ Submitting the form *schedules* a navigation to 'click.html'.
+ 2. We execute the anchor activation code which *navigates* to 'href.html'. The navigation to
+ 'href' is supposed to cancel the pending navigation to 'click.html' and we should navigate
+ to 'href.html', which is what the test asserts.
+
+ The issue for us is that we do not cancel pending navigations until after the navigation
+ policy decision is made, when the provisional loads actually starts, in FrameLoader::provisionalLoadStarted().
+ Because the policy decision for the navigation can now be made asynchronously, the NavigationScheduler
+ timer can now fire while the decision is made and we'll submit the form, thus navigating to
+ 'click.html'.
+
+ To address the issue, we now cancel any pending navigations in FrameLoader::loadWithDocumentLoader(),
+ *before* doing the policy check for the navigation.
+
+ Test: http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::loadWithDocumentLoader):
+
2018-02-28 Zalan Bujtas <[email protected]>
Remove RenderElement::s_affectsParentBlock
Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/loader/FrameLoader.cpp (229259 => 229260)
--- releases/WebKitGTK/webkit-2.20/Source/WebCore/loader/FrameLoader.cpp 2018-03-05 12:34:12 UTC (rev 229259)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/loader/FrameLoader.cpp 2018-03-05 12:34:23 UTC (rev 229260)
@@ -1527,6 +1527,8 @@
}
}
+ m_frame.navigationScheduler().cancel(true);
+
policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, loader, formState, [this, allowNavigationToInvalidURL] (const ResourceRequest& request, FormState* formState, bool shouldContinue) {
continueLoadAfterNavigationPolicy(request, formState, shouldContinue, allowNavigationToInvalidURL);
});
