Skip to content

Commit 5a57a21

Browse files
Stop duplicating history with streaming-enhanced-form PRG. Fixes #51674 (#53301)
# Stop duplicating history with streaming-enhanced-form PRG Makes the post-redirect-get pattern work without duplicating history entries. ## Description The PRG pattern is common but has the annoying disbenefit that, after submitting a form, the browser's "back" button doesn't work as normal because there is a duplicated history entry. The user has to click "back" twice or more times depending on how many times they have submitted the form. In the case of enhanced navigation, Blazor fixes this by explicitly avoiding duplicated history entries. But this wasn't implemented for the streaming+enhanced case. This PR adds the avoid-duplication behavior in that case too. Fixes #51674 ## Customer Impact Without this, the PRG pattern would work nicely on an enhanced form, but if you then also enabled streaming SSR for that form, it would go back to duplicating history entries and forcing the user to click "back" multiple times. **NOTE**: This PR still doesn't implement non-duplication in the non-enhanced-nav case (with or without streaming). That's out of scope because without enhanced nav, our code doesn't control how the response is handled, so we have to accept the default browser behavior (which does duplicate the history entries after PRG). ## Regression? - [ ] Yes - [x] No [If yes, specify the version the behavior has regressed from] ## Risk - [ ] High - [x] Medium - [ ] Low There's some pretty intricate logic for the interactions between streaming, enhanced nav, redirections, and the history stack. This adds another case. Mitigations: * We have pretty extensive tests for all the combinations we can think of * The way the change is structured, it *should* not impact other cases ## Verification - [x] Manual (required) - [x] Automated ## Packaging changes reviewed? - [ ] Yes - [ ] No - [x] N/A
1 parent 55c2632 commit 5a57a21

File tree

5 files changed

+91
-18
lines changed

5 files changed

+91
-18
lines changed

src/Components/Web.JS/dist/Release/blazor.web.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Components/Web.JS/src/Rendering/StreamingRendering.ts

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,19 +53,24 @@ class BlazorStreamingUpdate extends HTMLElement {
5353
const isFormPost = node.getAttribute('from') === 'form-post';
5454
const isEnhancedNav = node.getAttribute('enhanced') === 'true';
5555
if (isEnhancedNav && isWithinBaseUriSpace(destinationUrl)) {
56+
// At this point the destinationUrl might be an opaque URL so we don't know whether it's internal/external or
57+
// whether it's even going to the same URL we're currently on. So we don't know how to update the history.
58+
// Defer that until the redirection is resolved by performEnhancedPageLoad.
59+
const treatAsRedirectionFromMethod = isFormPost ? 'post' : 'get';
60+
const fetchOptions = undefined;
61+
performEnhancedPageLoad(destinationUrl, /* interceptedLink */ false, fetchOptions, treatAsRedirectionFromMethod);
62+
} else {
5663
if (isFormPost) {
5764
// The URL is not yet updated. Push a whole new entry so that 'back' goes back to the pre-redirection location.
58-
history.pushState(null, '', destinationUrl);
65+
// WARNING: The following check to avoid duplicating history entries won't work if the redirection is to an opaque URL.
66+
// We could change the server-side logic to return URLs in plaintext if they match the current request URL already,
67+
// but it's arguably easier to understand that history non-duplication only works for enhanced nav, which is also the
68+
// case for non-streaming responses.
69+
if (destinationUrl !== location.href) {
70+
location.assign(destinationUrl);
71+
}
5972
} else {
6073
// The URL was already updated on the original link click. Replace so that 'back' goes to the pre-redirection location.
61-
history.replaceState(null, '', destinationUrl);
62-
}
63-
performEnhancedPageLoad(destinationUrl, /* interceptedLink */ false);
64-
} else {
65-
// Same reason for varying as above
66-
if (isFormPost) {
67-
location.assign(destinationUrl);
68-
} else {
6974
location.replace(destinationUrl);
7075
}
7176
}

src/Components/Web.JS/src/Services/NavigationEnhancement.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,10 @@ function onDocumentSubmit(event: SubmitEvent) {
132132

133133
event.preventDefault();
134134

135-
const url = new URL(event.submitter?.getAttribute('formaction') || formElem.action, document.baseURI);
136-
const fetchOptions: RequestInit = { method: method};
135+
const url = new URL(event.submitter?.getAttribute('formaction') || formElem.action, document.baseURI);
136+
const fetchOptions: RequestInit = { method: method};
137137
const formData = new FormData(formElem);
138-
138+
139139
const submitterName = event.submitter?.getAttribute('name');
140140
const submitterValue = event.submitter!.getAttribute('value');
141141
if (submitterName && submitterValue) {
@@ -153,8 +153,8 @@ function onDocumentSubmit(event: SubmitEvent) {
153153
} else {
154154
// Setting request body and content-type header depending on enctype
155155
const enctype = event.submitter?.getAttribute('formenctype') || formElem.enctype;
156-
if (enctype === 'multipart/form-data') {
157-
// Content-Type header will be set to 'multipart/form-data'
156+
if (enctype === 'multipart/form-data') {
157+
// Content-Type header will be set to 'multipart/form-data'
158158
fetchOptions.body = formData;
159159
} else {
160160
fetchOptions.body = urlSearchParams;
@@ -170,7 +170,7 @@ function onDocumentSubmit(event: SubmitEvent) {
170170
}
171171
}
172172

173-
export async function performEnhancedPageLoad(internalDestinationHref: string, interceptedLink: boolean, fetchOptions?: RequestInit) {
173+
export async function performEnhancedPageLoad(internalDestinationHref: string, interceptedLink: boolean, fetchOptions?: RequestInit, treatAsRedirectionFromMethod?: 'get' | 'post') {
174174
performingEnhancedPageLoad = true;
175175

176176
// First, stop any preceding enhanced page load
@@ -232,8 +232,9 @@ export async function performEnhancedPageLoad(internalDestinationHref: string, i
232232
// For 301/302/etc redirections to internal URLs, the browser will already have followed the chain of redirections
233233
// to the end, and given us the final content. We do still need to update the current URL to match the final location,
234234
// then let the rest of enhanced nav logic run to patch the new content into the DOM.
235-
if (response.redirected) {
236-
if (isGetRequest) {
235+
if (response.redirected || treatAsRedirectionFromMethod) {
236+
const treatAsGet = treatAsRedirectionFromMethod ? (treatAsRedirectionFromMethod === 'get') : isGetRequest;
237+
if (treatAsGet) {
237238
// For gets, the intermediate (redirecting) URL is already in the address bar, so we have to use 'replace'
238239
// so that 'back' would go to the page before the redirection
239240
history.replaceState(null, '', response.url);

src/Components/test/E2ETest/ServerRenderingTests/FormHandlingTests/FormWithParentBindingContextTest.cs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1466,6 +1466,8 @@ public void SubmitButtonFormenctypeAttributeOverridesEnhancedFormEnctype()
14661466
[Fact]
14671467
public void EnhancedFormThatCallsNavigationManagerRefreshDoesNotPushHistoryEntry()
14681468
{
1469+
GoTo("about:blank");
1470+
14691471
var startUrl = Browser.Url;
14701472
GoTo("forms/form-that-calls-navigation-manager-refresh");
14711473
var guid = Browser.Exists(By.Id("guid")).Text;
@@ -1482,6 +1484,30 @@ public void EnhancedFormThatCallsNavigationManagerRefreshDoesNotPushHistoryEntry
14821484
Browser.Navigate().Back();
14831485
Browser.Equal(startUrl, () => Browser.Url);
14841486
}
1487+
1488+
[Fact]
1489+
public void EnhancedFormThatCallsNavigationManagerRefreshDoesNotPushHistoryEntry_Streaming()
1490+
{
1491+
GoTo("about:blank");
1492+
1493+
var startUrl = Browser.Url;
1494+
GoTo("forms/form-that-calls-navigation-manager-refresh-streaming");
1495+
1496+
// Submit the form
1497+
Browser.FindElement(By.Id("some-text")).SendKeys("test string");
1498+
Browser.Equal("test string", () => Browser.FindElement(By.Id("some-text")).GetAttribute("value"));
1499+
Browser.Exists(By.Id("submit-button")).Click();
1500+
1501+
// Wait for the async/streaming process to complete. We know this happened
1502+
// if the loading indicator says we're done, and the textbox was cleared
1503+
// due to the refresh
1504+
Browser.Equal("False", () => Browser.FindElement(By.Id("loading-indicator")).Text);
1505+
Browser.Equal("", () => Browser.FindElement(By.Id("some-text")).GetAttribute("value"));
1506+
1507+
// Checking that the history entry was not pushed
1508+
Browser.Navigate().Back();
1509+
Browser.Equal(startUrl, () => Browser.Url);
1510+
}
14851511

14861512
// Can't just use GetAttribute or GetDomAttribute because they both auto-resolve it
14871513
// to an absolute URL. We want to be able to assert about the attribute's literal value.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
@page "/forms/form-that-calls-navigation-manager-refresh-streaming"
2+
@using Microsoft.AspNetCore.Components.Forms
3+
@attribute [StreamRendering]
4+
@inject NavigationManager Nav
5+
6+
<h3>Form That Calls NavigationManager.Refresh() with streaming</h3>
7+
8+
<form data-enhance @onsubmit="@RefreshAfterDelayAsync" @formname="form-refresh" method="post">
9+
<AntiforgeryToken />
10+
<input id="some-text" name="SomeText" value="@SomeText" />
11+
<button id="submit-button">Submit</button>
12+
</form>
13+
14+
<p>Loading: <span id="loading-indicator">@loading</span></p>
15+
16+
@if (missingText)
17+
{
18+
<p>Enter some text, so you can see it go back to blank after the refresh happened.</p>
19+
}
20+
21+
@code {
22+
[SupplyParameterFromForm]
23+
public string SomeText { get; set; }
24+
25+
bool loading;
26+
bool missingText;
27+
28+
async Task RefreshAfterDelayAsync()
29+
{
30+
if (string.IsNullOrEmpty(SomeText))
31+
{
32+
missingText = true;
33+
}
34+
else
35+
{
36+
loading = true;
37+
await Task.Delay(1000);
38+
Nav.Refresh();
39+
}
40+
}
41+
}

0 commit comments

Comments
 (0)