Unexpected DOM persistence: Omitted attributes not removed during re-render#67663
Open
Vinoth2562000 wants to merge 9 commits into
Open
Unexpected DOM persistence: Omitted attributes not removed during re-render#67663Vinoth2562000 wants to merge 9 commits into
Vinoth2562000 wants to merge 9 commits into
Conversation
Refines the previous fix to address a logic flaw where the reset trigger fired for any parameter in the ParameterView, including cascading values. Previously, a re-render that only refreshed a cascading parameter (e.g. a CascadingEditContext whose value had not actually changed from the component's perspective) would have erroneously cleared the CaptureUnmatchedValues property. This could regress components like InputBase that rely on a stable AdditionalAttributes across re-renders triggered by cascading value changes. The reset is now gated on sawAnyNonCascadingParameter, which is set only when the parent supplied at least one direct (non-cascading) parameter, meaning the parent actually re-rendered the component with a new direct parameter set that no longer contains the previously-captured unmatched attributes. Also adds regression tests: - CaptureUnmatchedValues_IsResetToNull_WhenOnlyDirectParametersAreSupplied - CaptureUnmatchedValues_IsPreserved_WhenOnlyCascadingParametersAreSupplied - CaptureUnmatchedValues_IsPreserved_OnEmptyParameterView - CaptureUnmatchedValues_IsPreserved_WhenExplicitlySet_AndNoUnmatchedValues
The previous name described the iteration's discriminator (non-cascading) rather than its meaning in the surrounding logic. The flag actually answers the question 'did the parent re-render this component with a new direct parameter set?', which is what the reset decision depends on. parentSuppliedDirectParameters makes the intent obvious at the use site and pairs naturally with the existing isCaptureUnmatchedValuesParameterSetExplicitly.
Addresses the test coverage gaps identified in the Tech Lead's review of the CaptureUnmatchedValues / Issue 56463 fix: * TEST 4 (CRITICAL): real InputText end-to-end test that omits a splat attribute on a re-render and asserts a RemoveAttribute edit is generated. A new TestInputConditionalAttributeHostComponent mirrors the issue's Razor pattern. * TEST 2 (HIGH): three simultaneous splat attributes all removed in the same render. * TEST 5 (MEDIUM): HTML boolean attribute (disabled='') removal. * Edge cases: - mixed keep / change / remove across one render - multiple sibling components losing their splat attr together - rapid add-remove-add-remove cycle Cascading parameter interaction is already covered in ParameterViewTest.Assignment.cs.CaptureUnmatchedValues_IsPreserved_WhenOnlyCascadingParametersAreSupplied. Test results: Components.Tests: 1281 passed, 0 failed (+5 new) Web.Tests: 313 passed, 0 failed (+2 new) Forms.Tests: 188 passed, 0 failed
Adds RemovesUnmatchedAttributeFromParentAndChildIndependentlyInNestedHierarchy to cover TEST 10 from the TL review. Uses a new MyStrongContainerComponent that holds its own CaptureUnmatchedValues writer AND renders a child MyStrongComponent with its own. Both writers' diffs are verified independently in the second batch. Test results: Components.Tests 1282 passed, 0 failed.
Validation matrix (run with fix reverted vs with fix):
| Test | Pre-fix | With fix | Kept? |
|------------------------------------------------------|---------|----------|-------|
| RemovesSplatAttributeFromChildElementWhenOmitted... | FAIL | PASS | YES |
| RemovesOnlyOmittedUnmatchedAttributesFromChildElement| PASS | PASS | YES (pinned original repro) |
| CaptureUnmatchedValues_IsResetToNull_WhenNoUnmatched..| FAIL | PASS | YES |
| CaptureUnmatchedValues_IsPreserved_WhenOnlyCascading..| PASS | PASS | YES (pinned cascading bug) |
| RemovesBooleanUnmatchedAttributeFromChildElement... | FAIL | PASS | YES |
| RemovesMultipleUnmatchedAttributesFromChildElement...| FAIL | PASS | YES |
| RemovesUnmatchedAttributesAcrossMultipleChildComp... | FAIL | PASS | YES |
| RemovesUnmatchedAttribute_AcrossRapidAddRemoveCycles | FAIL | PASS | YES |
| RemovesUnmatchedAttributeFromParentAndChildInde... | FAIL | PASS | YES |
| InputText_RemovesAttributeFromChildren_WhenOmitted...| FAIL | PASS | YES |
Removed (5 tests):
* CaptureUnmatchedValues_RemainsNull_AfterSecondRenderWithNoUnmatchedValues
- target starts null and nothing assigns it non-null; the assertion
'is null' cannot fail
* CaptureUnmatchedValues_IsPreserved_OnEmptyParameterView
- with ParameterView.Empty the loop body never runs; the reset branch
is never reached regardless of the fix
* CaptureUnmatchedValues_IsPreserved_WhenExplicitlySet_AndNoUnmatchedValues
- exercises the explicit-set branch which is unchanged by the fix
* RemovesOnlyTheOmittedUnmatchedAttributesWhenOthersAreKeptOrChanged
- duplicate of the pre-existing RemovesOnlyOmittedUnmatchedAttributesFromChildElement
* InputText_OmitsAttributeFromFirstRender_WhenNotSupplied
- tautological: an attribute never supplied cannot appear
Result: 8 of 8 kept new tests fail without the fix and pass with it.
Full Components.Tests/Web.Tests/Forms.Tests pass with the fix in place.
The fix has no collateral damage on any unrelated test.
dotnet-policy-service
Bot
added
the
community-contribution
Contributor
|
Thanks for your PR, @Vinoth2562000. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unexpected DOM persistence: Omitted attributes not removed during re-render
Description
When a component or its parent dynamically rendered an attribute on a child element (typically via
[Parameter(CaptureUnmatchedValues = true)]on the child) and then omitted that attribute in a subsequent render, Blazor failed to remove the previously rendered attribute from the DOM. As a result, attributes that were no longer present in the render tree persisted in the browser's DOM, causing the rendered output to become inconsistent with the component state.To address this, the parameter assignment logic in
ComponentProperties.SetPropertieshas been updated to clear theCaptureUnmatchedValueswriter back tonullwhen the parent supplies at least one direct (non-cascading) parameter and does not explicitly set the capture property itself. This lets the renderer emit the missingRemoveAttributeedit and bring the rendered DOM back in sync with the parent's render output.Before
After
Changes included
SetPropertiesinComponentProperties.csto reset theCaptureUnmatchedValueswriter when the parent supplies a new direct-parameter set, so the renderer can correctly detect omitted attributes on subsequent renders.parentSuppliedDirectParametersflag, which ignores cascading parameters).Fixes #56463