Bug 1206879 - fire onlocationchange from pushState in frames when the URI changes, r=smaug

--HG-- extra : commitid : AuCEnWCXk6y extra : rebase_source : e825b13db3edccb115b7fdd2569154bbc434cfec
2015-09-29 13:50:55 -04:00 · 2015-09-29 13:50:55 -04:00 · 314c978904
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@ -11566,7 +11566,12 @@ nsDocShell::AddState(JS::Handle<JS::Value> aData, const nsAString& aTitle,
  // FireOnLocationChange(...) breaks security UI.
  if (!equalURIs) {
    document->SetDocumentURI(newURI);
-    SetCurrentURI(newURI, nullptr, true, LOCATION_CHANGE_SAME_DOCUMENT);
+    // We can't trust SetCurrentURI to do always fire locationchange events
+    // when we expect it to, so we hack around that by doing it ourselves...
+    SetCurrentURI(newURI, nullptr, false, LOCATION_CHANGE_SAME_DOCUMENT);
+    if (mLoadType != LOAD_ERROR_PAGE) {
+      FireDummyOnLocationChange();
+    }

    AddURIVisit(newURI, oldURI, oldURI, 0);

--- a/docshell/test/browser/browser.ini
+++ b/docshell/test/browser/browser.ini
@ -33,6 +33,7 @@ support-files =
  file_bug852909.pdf
  file_bug852909.png
  file_bug1046022.html
+  file_bug1206879.html
  file_multiple_pushState.html
  print_postdata.sjs
  test-form_sjis.html
@ -44,6 +45,7 @@ support-files =
  head.js
  frame-head.js

+[browser_bug1206879.js]
 [browser_bug134911.js]
 skip-if = e10s # Bug ?????? - BrowserSetForcedCharacterSet() in browser.js references docShell
 [browser_bug234628-1.js]
--- a/docshell/test/browser/browser_bug1206879.js
+++ b/docshell/test/browser/browser_bug1206879.js
@ -0,0 +1,31 @@
+add_task(function*() {
+  let url = getRootDirectory(gTestPath) + "file_bug1206879.html";
+  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, url, true);
+
+  let numLocationChanges = 0;
+
+  let listener = {
+    onLocationChange: function(browser, wp, request, uri, flags) {
+      if (browser != tab.linkedBrowser) {
+        return;
+      }
+      info("onLocationChange: " + uri.spec);
+      numLocationChanges++;
+      this.resolve();
+    }
+  };
+  let locationPromise = new Promise((resolve, reject) => {
+    listener.resolve = resolve;
+    gBrowser.addTabsProgressListener(listener);
+  });
+  yield ContentTask.spawn(tab.linkedBrowser, {}, function() {
+    content.frames[0].history.pushState(null, null, "foo");
+  });
+
+  yield locationPromise;
+
+  gBrowser.removeTab(tab);
+  gBrowser.removeTabsProgressListener(listener);
+  is(numLocationChanges, 1,
+     "pushState with a different URI should cause a LocationChange event.");
+});
--- a/docshell/test/browser/browser_bug554155.js
+++ b/docshell/test/browser/browser_bug554155.js
@ -9,7 +9,8 @@ function test() {
    let numLocationChanges = 0;

    let listener = {
-      onLocationChange: function() {
+      onLocationChange: function(browser, webProgress, request, uri, flags) {
+        info("location change: " + (uri && uri.spec));
        numLocationChanges++;
      }
    };
--- a/docshell/test/browser/file_bug1206879.html
+++ b/docshell/test/browser/file_bug1206879.html
@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <title>Test page for bug 1206879</title>
+  </head>
+  <body>
+    <iframe src="http://example.com/"></iframe>
+  </body>
+</html>