Skip to content

[Blazor] Emit action attribute when not explicit. #51130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Components/Samples/BlazorUnitedApp/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
app.UseHttpsRedirection();

app.UseStaticFiles();
app.UseAntiforgery();

app.MapRazorComponents<App>();

Expand Down
2 changes: 1 addition & 1 deletion src/Components/Web.JS/dist/Release/blazor.web.js

Large diffs are not rendered by default.

39 changes: 36 additions & 3 deletions src/Components/Web.JS/src/Services/NavigationEnhancement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ let currentEnhancedNavigationAbortController: AbortController | null;
let navigationEnhancementCallbacks: NavigationEnhancementCallbacks;
let performingEnhancedPageLoad: boolean;

// This gets initialized to the current URL when we load.
// After that, it gets updated every time we successfully complete a navigation.
let currentContentUrl = location.href;

export interface NavigationEnhancementCallbacks {
documentUpdated: () => void;
enhancedNavigationCompleted: () => void;
Expand Down Expand Up @@ -217,10 +221,25 @@ export async function performEnhancedPageLoad(internalDestinationHref: string, f
return;
}

if (!response.redirected && !isGetRequest && response.url !== location.href && isSuccessResponse) {
isNonRedirectedPostToADifferentUrlMessage = `Cannot perform enhanced form submission that changes the URL (except via a redirection), because then back/forward would not work. Either remove this form\'s \'action\' attribute, or change its method to \'get\', or do not mark it as enhanced.\nOld URL: ${location.href}\nNew URL: ${response.url}`;
if (!response.redirected && !isGetRequest && isSuccessResponse) {
// If this is the result of a form post that didn't trigger a redirection.
if (!isForSamePath(response)) {
// In this case we don't want to push the currentContentUrl to the history stack because we don't know if this is a location
// we can navigate back to (as we don't know if the location supports GET) and we are not able to replicate the Resubmit form?
// browser behavior.
// The only case where this is acceptable is when the last content URL, is the same as the URL for the form we posted to.
isNonRedirectedPostToADifferentUrlMessage = `Cannot perform enhanced form submission that changes the URL (except via a redirection), because then back/forward would not work. Either remove this form\'s \'action\' attribute, or change its method to \'get\', or do not mark it as enhanced.\nOld URL: ${location.href}\nNew URL: ${response.url}`;
} else {
if (location.href !== currentContentUrl) {
// The url on the browser might be out of data, so push an entry to the stack to update the url in place.
history.pushState(null, '', currentContentUrl);
}
}
}

// Set the currentContentUrl to the location of the last completed navigation.
currentContentUrl = response.url;

const responseContentType = response.headers.get('content-type');
if (responseContentType?.startsWith('text/html') && initialContent) {
// For HTML responses, regardless of the status code, display it
Expand Down Expand Up @@ -277,6 +296,20 @@ export async function performEnhancedPageLoad(internalDestinationHref: string, f
throw new Error(isNonRedirectedPostToADifferentUrlMessage);
}
}

function isForSamePath(response: Response) {
// We are trying to determine if the response URL is compatible with the last content URL that was successfully loaded on to
// the page.
// We are going to use the scheme, host, port and path to determine if they are compatible. We do not account for the query string
// as we want to allow for the query string to change. (Blazor doesn't use the query string for routing purposes).

const responseUrl = new URL(response.url);
const currentContentUrlParsed = new URL(currentContentUrl!);
return responseUrl.protocol === currentContentUrlParsed.protocol
&& responseUrl.host === currentContentUrlParsed.host
&& responseUrl.port === currentContentUrlParsed.port
&& responseUrl.pathname === currentContentUrlParsed.pathname;
}
}

async function getResponsePartsWithFraming(responsePromise: Promise<Response>, abortSignal: AbortSignal, onInitialDocument: (response: Response, initialDocumentText: string) => void, onStreamingElement: (streamingElementMarkup) => void) {
Expand Down Expand Up @@ -369,7 +402,7 @@ function enhancedNavigationIsEnabledForLink(element: HTMLAnchorElement): boolean
function enhancedNavigationIsEnabledForForm(form: HTMLFormElement): boolean {
// For forms, they default *not* to being enhanced, and must be enabled explicitly on the form element itself (not an ancestor).
const attributeValue = form.getAttribute('data-enhance');
return typeof(attributeValue) === 'string'
return typeof (attributeValue) === 'string'
&& attributeValue === '' || attributeValue?.toLowerCase() === 'true';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,9 @@ private int RenderElement(int componentId, TextWriter output, ArrayRange<RenderT
output.Write(frame.ElementName);
int afterElement;
var isTextArea = string.Equals(frame.ElementName, "textarea", StringComparison.OrdinalIgnoreCase);
var isForm = string.Equals(frame.ElementName, "form", StringComparison.OrdinalIgnoreCase);
// We don't want to include value attribute of textarea element.
var afterAttributes = RenderAttributes(output, frames, position + 1, frame.ElementSubtreeLength - 1, !isTextArea, out var capturedValueAttribute);
var afterAttributes = RenderAttributes(output, frames, position + 1, frame.ElementSubtreeLength - 1, !isTextArea, isForm: isForm, out var capturedValueAttribute);

// When we see an <option> as a descendant of a <select>, and the option's "value" attribute matches the
// "value" attribute on the <select>, then we auto-add the "selected" attribute to that option. This is
Expand Down Expand Up @@ -270,15 +271,23 @@ private static bool TryFindEnclosingElementFrame(ArrayRange<RenderTreeFrame> fra
}

private int RenderAttributes(
TextWriter output, ArrayRange<RenderTreeFrame> frames, int position, int maxElements, bool includeValueAttribute, out string? capturedValueAttribute)
TextWriter output,
ArrayRange<RenderTreeFrame> frames,
int position,
int maxElements,
bool includeValueAttribute,
bool isForm,
out string? capturedValueAttribute)
{
capturedValueAttribute = null;

if (maxElements == 0)
{
EmitFormActionIfNotExplicit(output, isForm, hasExplicitActionValue: false);
return position;
}

var hasExplicitActionValue = false;
for (var i = 0; i < maxElements; i++)
{
var candidateIndex = position + i;
Expand All @@ -291,6 +300,7 @@ private int RenderAttributes(
continue;
}

EmitFormActionIfNotExplicit(output, isForm, hasExplicitActionValue);
return candidateIndex;
}

Expand All @@ -304,6 +314,12 @@ private int RenderAttributes(
}
}

if (isForm && frame.AttributeName.Equals("action", StringComparison.OrdinalIgnoreCase) &&
!string.IsNullOrEmpty(frame.AttributeValue as string))
{
hasExplicitActionValue = true;
}

switch (frame.AttributeValue)
{
case bool flag when flag:
Expand All @@ -323,7 +339,22 @@ private int RenderAttributes(
}
}

EmitFormActionIfNotExplicit(output, isForm, hasExplicitActionValue);

return position + maxElements;

void EmitFormActionIfNotExplicit(TextWriter output, bool isForm, bool hasExplicitActionValue)
{
if (isForm && _navigationManager != null && !hasExplicitActionValue)
{
output.Write(' ');
output.Write("action");
output.Write('=');
output.Write('\"');
_htmlEncoder.Encode(output, _navigationManager.Uri);
output.Write('\"');
}
}
}

private int RenderChildren(int componentId, TextWriter output, ArrayRange<RenderTreeFrame> frames, int position, int maxElements)
Expand Down
3 changes: 3 additions & 0 deletions src/Components/Web/src/HtmlRendering/StaticHtmlRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Microsoft.AspNetCore.Components.RenderTree;
using Microsoft.AspNetCore.Components.Web;
using Microsoft.AspNetCore.Components.Web.HtmlRendering;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;

namespace Microsoft.AspNetCore.Components.HtmlRendering.Infrastructure;
Expand All @@ -18,6 +19,7 @@ namespace Microsoft.AspNetCore.Components.HtmlRendering.Infrastructure;
public partial class StaticHtmlRenderer : Renderer
{
private static readonly Task CanceledRenderTask = Task.FromCanceled(new CancellationToken(canceled: true));
private readonly NavigationManager? _navigationManager;

/// <summary>
/// Constructs an instance of <see cref="StaticHtmlRenderer"/>.
Expand All @@ -27,6 +29,7 @@ public partial class StaticHtmlRenderer : Renderer
public StaticHtmlRenderer(IServiceProvider serviceProvider, ILoggerFactory loggerFactory)
: base(serviceProvider, loggerFactory)
{
_navigationManager = serviceProvider.GetService<NavigationManager>();
}

/// <inheritdoc/>
Expand Down
Loading