Skip to content

Preserve TagHelper child content when appending output content#67087

Open
Wezylnia wants to merge 1 commit into
dotnet:mainfrom
Wezylnia:fix-taghelper-output-append-child-content
Open

Preserve TagHelper child content when appending output content#67087
Wezylnia wants to merge 1 commit into
dotnet:mainfrom
Wezylnia:fix-taghelper-output-append-child-content

Conversation

@Wezylnia

@Wezylnia Wezylnia commented Jun 8, 2026

Copy link
Copy Markdown

Fixes #29835

Summary

  • initialize TagHelperOutput.Content from cached child content so AppendHtml appends instead of replacing unmaterialized child markup
  • keep plain Content reads from marking content as modified
  • preserve replace semantics for SetContent/SetHtmlContent, suppress output behavior, and reinitialization reuse
  • add focused TagHelperOutput regression tests

Testing

  • Not run: local repo restore artifacts are unavailable without running restore.cmd; dotnet test with the installed .NET 10 SDK reaches MSBuild but fails on missing artifacts/bin/GenerateFiles/Directory.Build.props.

Copilot AI review requested due to automatic review settings June 8, 2026 18:23
@github-actions github-actions Bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jun 8, 2026
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Jun 8, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Thanks for your PR, @Wezylnia. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR changes TagHelperOutput.Content to surface existing child content via a new wrapper that avoids marking content as modified on read, and adds tests to validate the new semantics.

Changes:

  • Update TagHelperOutput.Content to initialize from GetChildContentAsync() via a new TagHelperOutputContent wrapper.
  • Adjust Reinitialize / SuppressOutput behavior around _content lifecycle.
  • Add unit tests covering Content read/append/set/suppress/reinitialize scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/Razor/Razor/test/TagHelpers/TagHelperOutputTest.cs Adds tests validating new Content behavior (append/read/set/suppress/reinitialize).
src/Razor/Razor/src/TagHelpers/TagHelperOutput.cs Changes Content initialization and introduces TagHelperOutputContent wrapper to track modifications and preserve initial child content.
Comments suppressed due to low confidence (1)

src/Razor/Razor/src/TagHelpers/TagHelperOutput.cs:110

  • Content now eagerly blocks on GetChildContentAsync() via GetAwaiter().GetResult(). This is sync-over-async and also forces child-content evaluation even when callers only intend to overwrite content (e.g., output.Content.SetContent(...)), which can introduce unnecessary work and potential deadlocks in environments with a synchronization context. Consider making the child-content retrieval lazy inside TagHelperOutputContent (store the Task/factory and only block when the initial content is actually needed for reading or for append-merge), so SetContent/Clear can avoid evaluating child content entirely.
    public TagHelperContent Content
    {
        get
        {
            if (_content == null)
            {
                _content = new TagHelperOutputContent(GetChildContentAsync().GetAwaiter().GetResult());
            }

            return _content;
        }

Comment on lines +446 to +457
public override bool IsEmptyOrWhiteSpace
{
get
{
if (_isModified || _initialContent == null)
{
return _content.IsEmptyOrWhiteSpace;
}

return new DefaultTagHelperContent().SetHtmlContent(_initialContent).IsEmptyOrWhiteSpace;
}
}
Comment on lines +568 to +575
private void EnsureInitialContent()
{
if (!_isModified && _initialContent != null)
{
_content.AppendHtml(_initialContent);
_initialContent = null;
}
}
@Wezylnia

Wezylnia commented Jun 8, 2026

Copy link
Copy Markdown
Author

@dotnet-policy-service agree

@Wezylnia Wezylnia force-pushed the fix-taghelper-output-append-child-content branch from 3e921f3 to e897165 Compare June 8, 2026 18:28
@Wezylnia Wezylnia force-pushed the fix-taghelper-output-append-child-content branch from e897165 to 99552ea Compare June 10, 2026 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TagHelperContent.AppendHtml does not append but replace

2 participants