replaceState cause back/forward malfunction on html page with <base h…

…ref="/"> tag https://bugs.webkit.org/show_bug.cgi?id=182678 <rdar://problem/37517821> Patch by Sihui Liu <sihui_liu@apple.com> on 2018-03-07 Reviewed by Chris Dumez. Source/WebCore: replaceState should not change URL when the URL argument is NULL, but should change URL when the URL argument is an empty string. Test: http/tests/history/replacestate-no-url.html * page/History.cpp: (WebCore::History::urlForState): LayoutTests: * http/tests/history/replacestate-no-url-expected.txt: Added. * http/tests/history/replacestate-no-url.html: Added. Add layout test coverage. * fast/loader/stateobjects/pushstate-with-fragment-urls-and-hashchange-expected.txt: Rebaseline a layout test as empty string for URL is handled differently. Canonical link: https://commits.webkit.org/199083@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229375 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Mar 7, 2018 · d06fa9e13d7fe56679b6c2889b7619fb25c29728 · d06fa9e
1 parent 7b6607c
commit d06fa9e13d7fe56679b6c2889b7619fb25c29728
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,18 @@
+2018-03-07  Sihui Liu  <sihui_liu@apple.com>
+
+        replaceState cause back/forward malfunction on html page with <base href="/"> tag
+        https://bugs.webkit.org/show_bug.cgi?id=182678
+        <rdar://problem/37517821>
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/history/replacestate-no-url-expected.txt: Added.
+        * http/tests/history/replacestate-no-url.html: Added.
+        Add layout test coverage.
+
+        * fast/loader/stateobjects/pushstate-with-fragment-urls-and-hashchange-expected.txt:
+        Rebaseline a layout test as empty string for URL is handled differently.
+
 2018-03-07  Chris Dumez  <cdumez@apple.com>
 
         http/tests/misc/location-replace-crossdomain.html is failing with async policy delegates

diff --git a/...utTests/fast/loader/stateobjects/pushstate-with-fragment-urls-and-hashchange-expected.txt b/...utTests/fast/loader/stateobjects/pushstate-with-fragment-urls-and-hashchange-expected.txt
@@ -27,8 +27,9 @@ State popped with event null (type object) and last path component pushstate-wit
 Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html#otherhash
 State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html#hash
 Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html#hash
+State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html
+Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html
 State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html#
 Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html#
-State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html#
 State popped with event OriginalEntry (type string) and last path component pushstate-with-fragment-urls-and-hashchange.html
 
diff --git a/LayoutTests/http/tests/history/replacestate-no-url-expected.txt b/LayoutTests/http/tests/history/replacestate-no-url-expected.txt
@@ -0,0 +1,18 @@
+Tests that ReplaceState should not change document URL if URL argument is null.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Push item one
+Replace item one with null url
+Push item two
+Going back to item one
+PASS document.location.href is "http://127.0.0.1:8000/one"
+Replace item one with empty url
+Push item two
+Going back to item one
+PASS document.location.href is "http://127.0.0.1:8000/"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/history/replacestate-no-url.html b/LayoutTests/http/tests/history/replacestate-no-url.html
@@ -0,0 +1,44 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <base href="/">
+  <script src="/js-test-resources/js-test.js"></script>
+  <script>
+  jsTestIsAsync = true;
+  description('Tests that ReplaceState should not change document URL if URL argument is null.');
+
+  function testNullUrl() {
+    debug('Push item one');
+    history.pushState({}, 'page 1', '/one');
+    debug('Replace item one with null url');
+    history.replaceState({}, 'replaced page 1');
+    debug('Push item two');
+    history.pushState({}, 'page 2', '/two');
+    debug('Going back to item one');
+    history.back();
+  }
+
+  function testEmptyUrl() {
+    debug('Replace item one with empty url');
+    history.replaceState('StopEntry', 'replaced page 1 again', '');
+    debug('Push item two');
+    history.pushState({}, 'page 2', '/two');
+    debug('Going back to item one');
+    history.back();
+  }
+
+  window.onpopstate = function(event) {
+    if (event.state != 'StopEntry') {
+      shouldBeEqualToString('document.location.href', 'http://127.0.0.1:8000/one');
+      setTimeout(testEmptyUrl, 0);
+    } else {
+      shouldBeEqualToString('document.location.href', 'http://127.0.0.1:8000/');
+      finishJSTest();
+    }
+  }
+  </script>
+</head>
+
+<body onload="testNullUrl()">
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,18 @@
+2018-03-07  Sihui Liu  <sihui_liu@apple.com>
+
+        replaceState cause back/forward malfunction on html page with <base href="/"> tag
+        https://bugs.webkit.org/show_bug.cgi?id=182678
+        <rdar://problem/37517821>
+
+        Reviewed by Chris Dumez.
+
+        replaceState should not change URL when the URL argument is NULL, but should change URL when the URL argument is an empty string.
+
+        Test: http/tests/history/replacestate-no-url.html
+
+        * page/History.cpp:
+        (WebCore::History::urlForState):
+
 2018-03-07  Antti Koivisto  <antti@apple.com>
 
         Don't invalidate descendants for sibling combinators unless needed

diff --git a/Source/WebCore/page/History.cpp b/Source/WebCore/page/History.cpp
@@ -156,11 +156,9 @@ void History::go(Document& document, int distance)
 
 URL History::urlForState(const String& urlString)
 {
-    URL baseURL = m_frame->document()->baseURL();
-    if (urlString.isEmpty())
-        return baseURL;
-
-    return URL(baseURL, urlString);
+    if (urlString.isNull())
+        return m_frame->document()->url();
+    return m_frame->document()->completeURL(urlString);
 }
 
 ExceptionOr<void> History::stateObjectAdded(RefPtr<SerializedScriptValue>&& data, const String& title, const String& urlString, StateObjectType stateObjectType)