Skip to content

Commit

Permalink
Bug 1825745 - Make form submits do a replace load if they happen befo…
Browse files Browse the repository at this point in the history
…re document load has ended. r=smaug

Differential Revision: https://phabricator.services.mozilla.com/D174224
  • Loading branch information
petervanderbeken committed Jun 22, 2023
1 parent 2a5b096 commit 1e615c9
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 17 deletions.
65 changes: 55 additions & 10 deletions docshell/base/nsDocShell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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!
Expand Down Expand Up @@ -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!");
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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())
Expand Down
7 changes: 7 additions & 0 deletions docshell/base/nsDocShell.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 7 additions & 1 deletion docshell/test/mochitest/file_bug1773192_2.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
[form-requestsubmit.html]
expected:
if (os == "android") and fission: [OK, TIMEOUT]
[Replace before load, triggered by formElement.requestSubmit()]
expected: FAIL
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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]

0 comments on commit 1e615c9

Please sign in to comment.