Bug 1825745 - Make form submits do a replace load if they happen before document load has ended. r=smaug

Differential Revision: https://phabricator.services.mozilla.com/D174224

Author: petervanderbeken

docshell/base/nsDocShell.cpp

@@ -971,10 +971,21 @@ bool nsDocShell::MaybeHandleSubframeHistory(
       // executing an onLoad Handler,this load will not go
       // into session history.
       // XXX Why is this code in a method which deals with iframes!
-      bool inOnLoadHandler = false;
-      GetIsExecutingOnLoadHandler(&inOnLoadHandler);
-      if (inOnLoadHandler) {
-        aLoadState->SetLoadType(LOAD_NORMAL_REPLACE);
+      if (aLoadState->IsFormSubmission()) {
+#ifdef DEBUG
+        if (!mEODForCurrentDocument) {
+          const MaybeDiscarded<BrowsingContext>& targetBC =
+              aLoadState->TargetBrowsingContext();
+          MOZ_ASSERT_IF(GetBrowsingContext() == targetBC.get(),
+                        aLoadState->LoadType() == LOAD_NORMAL_REPLACE);
+        }
+#endif
+      } else {
+        bool inOnLoadHandler = false;
+        GetIsExecutingOnLoadHandler(&inOnLoadHandler);
+        if (inOnLoadHandler) {
+          aLoadState->SetLoadType(LOAD_NORMAL_REPLACE);
+        }
       }
     }
     return false;
@@ -8561,7 +8572,7 @@ nsresult nsDocShell::PerformRetargeting(nsDocShellLoadState* aLoadState) {
       // Explicit principal because we do not want any guesses as to what the
       // principal to inherit is: it should be aTriggeringPrincipal.
       loadState->SetPrincipalIsExplicit(true);
-      loadState->SetLoadType(LOAD_LINK);
+      loadState->SetLoadType(aLoadState->LoadType());
       loadState->SetForceAllowDataURI(aLoadState->HasInternalLoadFlags(
           INTERNAL_LOAD_FLAGS_FORCE_ALLOW_DATA_URI));
 
@@ -8613,6 +8624,11 @@ nsresult nsDocShell::PerformRetargeting(nsDocShellLoadState* aLoadState) {
   }
 
   aLoadState->SetTargetBrowsingContext(targetContext);
+  if (aLoadState->IsFormSubmission()) {
+    aLoadState->SetLoadType(
+        GetLoadTypeForFormSubmission(targetContext, aLoadState));
+  }
+
   //
   // Transfer the load to the target BrowsingContext... Clear the window target
   // name to the empty string to prevent recursive retargeting!
@@ -9226,6 +9242,20 @@ static bool NavigationShouldTakeFocus(nsDocShell* aDocShell,
   return !Preferences::GetBool("browser.tabs.loadDivertedInBackground", false);
 }
 
+uint32_t nsDocShell::GetLoadTypeForFormSubmission(
+    BrowsingContext* aTargetBC, nsDocShellLoadState* aLoadState) {
+  MOZ_ASSERT(aLoadState->IsFormSubmission());
+
+  // https://html.spec.whatwg.org/#form-submission-algorithm
+  //  22. Let historyHandling be "push".
+  //  23. If form document equals targetNavigable's active document, and
+  //      form document has not yet completely loaded, then set
+  //      historyHandling to "replace".
+  return GetBrowsingContext() == aTargetBC && !mEODForCurrentDocument
+             ? LOAD_NORMAL_REPLACE
+             : LOAD_LINK;
+}
+
 nsresult nsDocShell::InternalLoad(nsDocShellLoadState* aLoadState,
                                   Maybe<uint32_t> aCacheKey) {
   MOZ_ASSERT(aLoadState, "need a load state!");
@@ -9265,6 +9295,8 @@ nsresult nsDocShell::InternalLoad(nsDocShellLoadState* aLoadState,
     return PerformRetargeting(aLoadState);
   }
 
+  // This is the non-retargeting load path, we've already set the right loadtype
+  // for form submissions in nsDocShell::OnLinkClickSync.
   if (aLoadState->TargetBrowsingContext().IsNull()) {
     aLoadState->SetTargetBrowsingContext(GetBrowsingContext());
   }
@@ -13084,11 +13116,24 @@ nsresult nsDocShell::OnLinkClickSync(nsIContent* aContent,
     CopyUTF8toUTF16(type, typeHint);
   }
 
-  // Link click (or form submission) can be triggered inside an onload
-  // handler, and we don't want to add history entry in this case.
-  bool inOnLoadHandler = false;
-  GetIsExecutingOnLoadHandler(&inOnLoadHandler);
-  uint32_t loadType = inOnLoadHandler ? LOAD_NORMAL_REPLACE : LOAD_LINK;
+  uint32_t loadType = LOAD_LINK;
+  if (aLoadState->IsFormSubmission()) {
+    if (aLoadState->Target().IsEmpty()) {
+      // We set the right load type here for form submissions with an empty
+      // target. Form submission with a non-empty target are handled in
+      // nsDocShell::PerformRetargeting after we've selected the correct target
+      // BC.
+      loadType = GetLoadTypeForFormSubmission(GetBrowsingContext(), aLoadState);
+    }
+  } else {
+    // Link click can be triggered inside an onload handler, and we don't want
+    // to add history entry in this case.
+    bool inOnLoadHandler = false;
+    GetIsExecutingOnLoadHandler(&inOnLoadHandler);
+    if (inOnLoadHandler) {
+      loadType = LOAD_NORMAL_REPLACE;
+    }
+  }
 
   nsCOMPtr<nsIReferrerInfo> referrerInfo =
       elementCanHaveNoopener ? new ReferrerInfo(*aContent->AsElement())

docshell/base/nsDocShell.h

@@ -710,6 +710,13 @@ class nsDocShell final : public nsDocLoader,
   nsresult ScrollToAnchor(bool aCurHasRef, bool aNewHasRef,
                           nsACString& aNewHash, uint32_t aLoadType);
 
+  // This returns the load type for a form submission (see
+  // https://html.spec.whatwg.org/#form-submission-algorithm). The load type
+  // should be set as soon as the target BC has been determined.
+  uint32_t GetLoadTypeForFormSubmission(
+      mozilla::dom::BrowsingContext* aTargetBC,
+      nsDocShellLoadState* aLoadState);
+
  private:
   // Returns true if would have called FireOnLocationChange,
   // but did not because aFireOnLocationChange was false on entry.

docshell/test/mochitest/file_bug1773192_2.html

@@ -7,7 +7,13 @@
     <form method="POST" action="file_bug1773192_3.sjs"></form>
     <script>
       history.replaceState({}, "", document.referrer);
-      document.forms[0].submit();
+      setTimeout(() => {
+        // The test relies on this page not going into the BFCache, so that
+        // when we come back to it we load the URL from the replaceState
+        // instead.
+        window.blockBFCache = new RTCPeerConnection();
+        document.forms[0].submit();
+      }, 0);
     </script>
   </body>
 </html>

testing/web-platform/meta/html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/form-requestsubmit.html.ini

@@ -1,5 +1,3 @@
 [form-requestsubmit.html]
   expected:
     if (os == "android") and fission: [OK, TIMEOUT]
-  [Replace before load, triggered by formElement.requestSubmit()]
-    expected: FAIL

testing/web-platform/meta/html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/form-submit-button-click.html.ini

@@ -1,5 +1,3 @@
 [form-submit-button-click.html]
   expected:
     if (os == "android") and fission: [OK, TIMEOUT]
-  [Replace before load, triggered by submitButton.click()]
-    expected: FAIL

testing/web-platform/meta/html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/form-submit.html.ini

@@ -1,3 +1,3 @@
 [form-submit.html]
-  [Replace before load, triggered by same-document formElement.submit()]
-    expected: FAIL
+  expected:
+    if (os == "android") and fission: [OK, TIMEOUT]