REGRESSION: fast/dom/cssTarget-crash.html fails
authoryael.aharon@nokia.com <yael.aharon@nokia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Dec 2010 19:31:00 +0000 (19:31 +0000)
committeryael.aharon@nokia.com <yael.aharon@nokia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Dec 2010 19:31:00 +0000 (19:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=20342

Reviewed by Adam Barth.

WebCore:

Do not reload the page when submitting a form, using "GET" method, and the
form action url matches the location url, except for the fragment.

Test: fast/forms/submit-change-fragment.html

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadURL):
(WebCore::FrameLoader::loadWithDocumentLoader):
(WebCore::FrameLoader::shouldScrollToAnchor):
* loader/FrameLoader.h:

LayoutTests:

* fast/forms/submit-change-fragment-expected.txt: Added.
* fast/forms/submit-change-fragment.html: Added.
* platform/mac/Skipped:
* platform/qt/Skipped:

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

LayoutTests/ChangeLog
LayoutTests/fast/forms/submit-change-fragment-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/submit-change-fragment.html [new file with mode: 0644]
LayoutTests/platform/mac/Skipped
LayoutTests/platform/qt/Skipped
WebCore/ChangeLog
WebCore/loader/FrameLoader.cpp
WebCore/loader/FrameLoader.h

index 484d933..5b4a118 100644 (file)
@@ -1,3 +1,15 @@
+2010-12-30  Yael Aharon  <yael.aharon@nokia.com>
+
+        Reviewed by Adam Barth.
+
+        REGRESSION: fast/dom/cssTarget-crash.html fails
+        https://bugs.webkit.org/show_bug.cgi?id=20342
+
+        * fast/forms/submit-change-fragment-expected.txt: Added.
+        * fast/forms/submit-change-fragment.html: Added.
+        * platform/mac/Skipped:
+        * platform/qt/Skipped:
+
 2010-12-30  Philippe Normand  <pnormand@igalia.com>
 
         Unreviewed, unskip the test, it's running fine locally.
diff --git a/LayoutTests/fast/forms/submit-change-fragment-expected.txt b/LayoutTests/fast/forms/submit-change-fragment-expected.txt
new file mode 100644 (file)
index 0000000..2a111af
--- /dev/null
@@ -0,0 +1,7 @@
+https://bugs.webkit.org/show_bug.cgi?id=20342
+Test that when the form method is get, and the form action is the same as the current location url, but with different fragment, we do not reload the page, and the onload handler is not called again.
+
+Also test that changing the form action after the form was submitted has no effect.
+
+
+PASS
diff --git a/LayoutTests/fast/forms/submit-change-fragment.html b/LayoutTests/fast/forms/submit-change-fragment.html
new file mode 100644 (file)
index 0000000..f055062
--- /dev/null
@@ -0,0 +1,37 @@
+<html>
+<script>
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+}
+
+if (document.location.href.indexOf("n=v") == -1)
+    document.location.search = "?n=v";
+
+function runTest() {
+    document.forms.f.action="#firstaction";
+    document.forms.f.submit();
+    document.forms.f.action="#secondaction";
+}
+
+function hashChanged() {
+    if (document.location.href.indexOf("firstaction") != -1) {
+        document.getElementById("console").innerHTML="PASS";
+        if (window.layoutTestController)
+            layoutTestController.notifyDone();
+    }
+}
+
+
+</script>
+<body onhashchange="hashChanged();" onload="runTest();">
+<div><a href="https://bugs.webkit.org/show_bug.cgi?id=20342">https://bugs.webkit.org/show_bug.cgi?id=20342</a></div>
+<p>Test that when the form method is get, and the form action is the same as the current location url, but with different fragment, we do not reload the page, and the onload handler is not called again.</p>
+<p>Also test that changing the form action after the form was submitted has no effect.</p>
+
+<form name="f" method="GET" action="#action"><input name="n" value="v"></form>
+<div id="console">FAIL</div>
+<script>
+</script>
+</body>
+</html>
index 05a8f02..2b5c7c8 100644 (file)
@@ -35,9 +35,6 @@ fast/events/attempt-scroll-with-no-scrollbars.html
 # Skip because fix for https://bugs.webkit.org/show_bug.cgi?id=26770 was reverted
 compositing/animation/animated-composited-inside-hidden.html
 
-# https://bugs.webkit.org/show_bug.cgi?id=20342 REGRESSION: fast/dom/cssTarget-crash.html fails
-fast/dom/cssTarget-crash.html
-
 # https://bugs.webkit.org/show_bug.cgi?id=21916 Pixel test doesn't repaint entire view so result is corrupted by previous test
 tables/mozilla_expected_failures/bugs/bug178855.xml
 
index e88f0a3..202442f 100644 (file)
@@ -2513,7 +2513,6 @@ fast/transforms/transform-positioned-ancestor.html
 # -- timedout with --platform mac --ignore-metrics
 fast/events/5056619.html
 fast/events/drag-in-frames.html
-fast/dom/cssTarget-crash.html
 fast/loader/null-request-after-willSendRequest.html
 fast/text/international/thai-line-breaks.html
 
index f4d244b..07d32e2 100644 (file)
@@ -1,3 +1,21 @@
+2010-12-30  Yael Aharon  <yael.aharon@nokia.com>
+
+        Reviewed by Adam Barth.
+
+        REGRESSION: fast/dom/cssTarget-crash.html fails
+        https://bugs.webkit.org/show_bug.cgi?id=20342
+
+        Do not reload the page when submitting a form, using "GET" method, and the
+        form action url matches the location url, except for the fragment.
+      
+        Test: fast/forms/submit-change-fragment.html
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::loadURL):
+        (WebCore::FrameLoader::loadWithDocumentLoader):
+        (WebCore::FrameLoader::shouldScrollToAnchor):
+        * loader/FrameLoader.h:
+
 2010-12-30  Darin Adler  <darin@apple.com>
 
         Reviewed by Adam Barth.
index ef692b6..eb09e57 100644 (file)
@@ -1319,11 +1319,12 @@ void FrameLoader::loadURL(const KURL& newURL, const String& referrer, const Stri
     RefPtr<DocumentLoader> oldDocumentLoader = m_documentLoader;
 
     bool sameURL = shouldTreatURLAsSameAsCurrent(newURL);
+    const String& httpMethod = request.httpMethod();
     
     // Make sure to do scroll to anchor processing even if the URL is
     // exactly the same so pages with '#' links and DHTML side effects
     // work properly.
-    if (shouldScrollToAnchor(isFormSubmission, newLoadType, newURL)) {
+    if (shouldScrollToAnchor(isFormSubmission, httpMethod, newLoadType, newURL)) {
         oldDocumentLoader->setTriggeringAction(action);
         policyChecker()->stopCheck();
         policyChecker()->setLoadType(newLoadType);
@@ -1442,8 +1443,9 @@ void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t
     bool isFormSubmission = formState;
 
     const KURL& newURL = loader->request().url();
+    const String& httpMethod = loader->request().httpMethod();
 
-    if (shouldScrollToAnchor(isFormSubmission, policyChecker()->loadType(), newURL)) {
+    if (shouldScrollToAnchor(isFormSubmission,  httpMethod, policyChecker()->loadType(), newURL)) {
         RefPtr<DocumentLoader> oldDocumentLoader = m_documentLoader;
         NavigationAction action(newURL, policyChecker()->loadType(), isFormSubmission);
 
@@ -2818,17 +2820,17 @@ void FrameLoader::continueFragmentScrollAfterNavigationPolicy(const ResourceRequ
     loadInSameDocument(request.url(), 0, !isRedirect);
 }
 
-bool FrameLoader::shouldScrollToAnchor(bool isFormSubmission, FrameLoadType loadType, const KURL& url)
+bool FrameLoader::shouldScrollToAnchor(bool isFormSubmission, const String& httpMethod, FrameLoadType loadType, const KURL& url)
 {
     // Should we do anchor navigation within the existing content?
 
-    // We don't do this if we are submitting a form, explicitly reloading,
+    // We don't do this if we are submitting a form with method other than "GET", explicitly reloading,
     // currently displaying a frameset, or if the URL does not have a fragment.
     // These rules were originally based on what KHTML was doing in KHTMLPart::openURL.
 
     // FIXME: What about load types other than Standard and Reload?
 
-    return !isFormSubmission
+    return (!isFormSubmission || equalIgnoringCase(httpMethod, "GET"))
         && loadType != FrameLoadTypeReload
         && loadType != FrameLoadTypeReloadFromOrigin
         && loadType != FrameLoadTypeSame
index 41a1615..f1c2189 100644 (file)
@@ -366,7 +366,7 @@ private:
     void continueLoadAfterNewWindowPolicy(const ResourceRequest&, PassRefPtr<FormState>, const String& frameName, const NavigationAction&, bool shouldContinue);
     void continueFragmentScrollAfterNavigationPolicy(const ResourceRequest&, bool shouldContinue);
 
-    bool shouldScrollToAnchor(bool isFormSubmission, FrameLoadType, const KURL&);
+    bool shouldScrollToAnchor(bool isFormSubmission, const String& httpMethod, FrameLoadType, const KURL&);
 
     void checkLoadCompleteForThisFrame();