Skip to content

Commit 1bbbdcf

Browse files
[main] Stop duplicating history with streaming-enhanced-form PRG. Fixes #51674 (#53303)
1 parent 6946e47 commit 1bbbdcf

File tree

4 files changed

+90
-17
lines changed

4 files changed

+90
-17
lines changed

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)