Skip to content

Commit 355e214

Browse files
csharpfritzCopilot
andcommitted
fix: address 5 audit findings for M17 AJAX controls
- ScriptManager: EnablePartialRendering now defaults to true (matches WF) - ScriptManager: Added Scripts collection (List<ScriptReference>) - UpdateProgress: CssClass now renders on output div element - UpdateProgress: Non-dynamic mode renders display:block;visibility:hidden - ScriptReference: Added ScriptMode, NotifyScriptLoaded, ResourceUICultures Tests: 9 new bUnit tests covering all 5 fixes (1367 total, 0 failures) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1e8b3a0 commit 355e214

9 files changed

Lines changed: 154 additions & 7 deletions

File tree

.ai-team/agents/cyclops/history.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,12 @@
4646

4747
Team update (2026-02-27): Timer duplicate [Parameter] bug fixed; 47 M17 tests established with C# API pattern for Timer decided by Rogue
4848

49+
### M17 Control Audit Fixes (2026-02-27)
50+
51+
- **Fix 1:** ScriptManager `EnablePartialRendering` default changed from `false` to `true` to match Web Forms default.
52+
- **Fix 2:** ScriptManager now has `Scripts` collection (`List<ScriptReference>`) matching ScriptManagerProxy pattern. Added `using System.Collections.Generic;`.
53+
- **Fix 3:** UpdateProgress `<div>` elements now render `class="@CssClass"` conditionally (only when CssClass is non-empty) to match Web Forms styled output.
54+
- **Fix 4:** UpdateProgress non-dynamic mode div style changed from `visibility:hidden;` to `display:block;visibility:hidden;` matching Web Forms explicit rendering.
55+
- **Fix 5:** ScriptReference gained `ScriptMode` (default Auto), `NotifyScriptLoaded` (default true), and `ResourceUICultures` properties from the Web Forms original. Added `using BlazorWebFormsComponents.Enums;`.
56+
- **Lesson:** Always verify default values against the Web Forms originals — bool properties default to `false` in C# but Web Forms often defaults them to `true`.
57+

.ai-team/agents/rogue/history.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,26 @@ Wrote 47 bUnit tests across 6 new test files for the M17 AJAX/migration-helper c
7171
📌 Test pattern: UpdateProgress DynamicLayout=true renders `display:none`, DynamicLayout=false renders `visibility:hidden`. Both use `<div>` wrapper. UpdatePanel Block mode renders `<div>`, Inline mode renders `<span>`. Both use `id="@ClientID"`. — Rogue
7272

7373
📌 Bug fix: Removed `new bool Enabled` `[Parameter]` from Timer.razor.cs — it shadowed BaseWebFormsComponent.Enabled causing a runtime InvalidOperationException (duplicate parameter name). Timer now uses the inherited Enabled property which has the same default (true). — Rogue
74+
75+
### M17 Audit Fix Tests
76+
77+
Added 9 new tests covering the 5 audit fixes from Forge's M17 audit:
78+
79+
**ScriptManagerTests.razor (6 new/updated tests):**
80+
- Updated `ScriptManager_DefaultEnablePartialRendering_IsTrue` — changed from `ShouldBeFalse()` to `ShouldBeTrue()` (Fix 1: default now matches Web Forms)
81+
- `ScriptManager_DefaultScripts_IsInitialized` — verifies Scripts collection is non-null and empty (Fix 2)
82+
- `ScriptManager_Scripts_CanHoldScriptReferences` — passes List<ScriptReference> with 2 items, verifies Name/Path (Fix 2)
83+
- `ScriptReference_DefaultScriptMode_IsAuto` — verifies ScriptMode defaults to Auto (Fix 5)
84+
- `ScriptReference_DefaultNotifyScriptLoaded_IsTrue` — verifies NotifyScriptLoaded defaults to true (Fix 5)
85+
- `ScriptReference_DefaultResourceUICultures_IsNull` — verifies ResourceUICultures defaults to null (Fix 5)
86+
87+
**UpdateProgressTests.razor (3 new/updated tests):**
88+
- Updated `UpdateProgress_DynamicLayoutFalse_UsesDisplayBlockVisibilityHidden` — renamed from `UsesVisibilityHidden`, now asserts both `display:block` and `visibility:hidden` (Fix 4)
89+
- `UpdateProgress_CssClass_AppliedToDiv` — sets CssClass="progress-overlay", verifies class attribute on div (Fix 3)
90+
- `UpdateProgress_NoCssClass_NoClassAttribute` — no CssClass set, verifies class attribute is null (Fix 3)
91+
92+
All 29 ScriptManager/UpdateProgress/ScriptReference tests pass (0 failures). Build has 60 pre-existing warnings (none from new tests).
93+
94+
📌 Test pattern: ScriptReference properties tested as plain C# object instantiation (no bUnit render needed) — `new ScriptReference()` then assert defaults. Same pattern as ScriptManagerProxy Scripts/Services tests. — Rogue
95+
96+
📌 Test pattern: UpdateProgress CssClass uses `class="@(string.IsNullOrEmpty(CssClass) ? null : CssClass)"` — when CssClass is empty/null, AngleSharp returns null for `GetAttribute("class")`, matching Web Forms behavior of omitting the class attribute entirely. — Rogue
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# M17 Audit Fixes — Cyclops
2+
3+
**Date:** 2026-02-27
4+
**Author:** Cyclops
5+
**Milestone:** M17
6+
7+
## Decisions
8+
9+
1. **ScriptManager.EnablePartialRendering defaults to `true`** — Matches Web Forms. Any test asserting `false` default should be updated.
10+
11+
2. **ScriptManager now has a `Scripts` collection** — Same `List<ScriptReference>` pattern as ScriptManagerProxy. Components consuming ScriptManager should be aware of this property.
12+
13+
3. **UpdateProgress renders `class` attribute conditionally** — Uses `string.IsNullOrEmpty(CssClass) ? null : CssClass` pattern so the `class` attribute is omitted entirely when no CssClass is set, keeping HTML clean.
14+
15+
4. **UpdateProgress non-dynamic mode uses `display:block;visibility:hidden;`** — Matches Web Forms explicit style rendering. Tests checking the non-dynamic div style should expect both properties.
16+
17+
5. **ScriptReference expanded with ScriptMode, NotifyScriptLoaded, ResourceUICultures** — These are migration-compatibility properties from `System.Web.UI.ScriptReference`. No behavioral impact since ScriptManager/ScriptManagerProxy are no-op stubs, but allows markup to transfer without errors.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
### M17 Audit Fix Test Coverage
2+
3+
**By:** Rogue
4+
**What:** Added 9 bUnit tests covering all 5 M17 audit fixes (EnablePartialRendering default, Scripts collection, CssClass rendering, display:block;visibility:hidden, ScriptReference properties). Updated 2 existing tests to match new behavior. All 29 ScriptManager/UpdateProgress tests pass.
5+
**Why:** Audit fixes change observable behavior — tests must be updated to assert the corrected defaults and new properties. ScriptReference defaults tested via plain C# instantiation (no render needed). UpdateProgress CssClass tested both with and without value to ensure no spurious `class=""` attribute.

src/BlazorWebFormsComponents.Test/ScriptManager/ScriptManagerTests.razor

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@
1414
}
1515

1616
[Fact]
17-
public void ScriptManager_DefaultEnablePartialRendering_IsFalse()
17+
public void ScriptManager_DefaultEnablePartialRendering_IsTrue()
1818
{
1919
var cut = Render(@<ScriptManager />);
2020
var sm = cut.FindComponent<ScriptManager>();
21-
sm.Instance.EnablePartialRendering.ShouldBeFalse();
21+
sm.Instance.EnablePartialRendering.ShouldBeTrue();
2222
}
2323

2424
[Fact]
@@ -91,4 +91,50 @@
9191
sm.Instance.ScriptMode.ShouldBe(ScriptMode.Release);
9292
sm.Instance.AsyncPostBackTimeout.ShouldBe(120);
9393
}
94+
95+
[Fact]
96+
public void ScriptManager_DefaultScripts_IsInitialized()
97+
{
98+
var cut = Render(@<ScriptManager />);
99+
var sm = cut.FindComponent<ScriptManager>();
100+
sm.Instance.Scripts.ShouldNotBeNull();
101+
sm.Instance.Scripts.Count.ShouldBe(0);
102+
}
103+
104+
[Fact]
105+
public void ScriptManager_Scripts_CanHoldScriptReferences()
106+
{
107+
var scripts = new System.Collections.Generic.List<ScriptReference>
108+
{
109+
new ScriptReference { Name = "jquery", Path = "~/scripts/jquery.js" },
110+
new ScriptReference { Name = "app", Path = "~/scripts/app.js" }
111+
};
112+
113+
var cut = Render(@<ScriptManager Scripts="scripts" />);
114+
var sm = cut.FindComponent<ScriptManager>();
115+
sm.Instance.Scripts.Count.ShouldBe(2);
116+
sm.Instance.Scripts[0].Name.ShouldBe("jquery");
117+
sm.Instance.Scripts[1].Path.ShouldBe("~/scripts/app.js");
118+
}
119+
120+
[Fact]
121+
public void ScriptReference_DefaultScriptMode_IsAuto()
122+
{
123+
var scriptRef = new ScriptReference();
124+
scriptRef.ScriptMode.ShouldBe(ScriptMode.Auto);
125+
}
126+
127+
[Fact]
128+
public void ScriptReference_DefaultNotifyScriptLoaded_IsTrue()
129+
{
130+
var scriptRef = new ScriptReference();
131+
scriptRef.NotifyScriptLoaded.ShouldBeTrue();
132+
}
133+
134+
[Fact]
135+
public void ScriptReference_DefaultResourceUICultures_IsNull()
136+
{
137+
var scriptRef = new ScriptReference();
138+
scriptRef.ResourceUICultures.ShouldBeNull();
139+
}
94140
}

src/BlazorWebFormsComponents.Test/UpdateProgress/UpdateProgressTests.razor

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
}
3939

4040
[Fact]
41-
public void UpdateProgress_DynamicLayoutFalse_UsesVisibilityHidden()
41+
public void UpdateProgress_DynamicLayoutFalse_UsesDisplayBlockVisibilityHidden()
4242
{
4343
// Arrange & Act
4444
var cut = Render(
@@ -51,6 +51,7 @@
5151

5252
// Assert
5353
var div = cut.Find("div#dynFalse");
54+
div.GetAttribute("style").ShouldContain("display:block");
5455
div.GetAttribute("style").ShouldContain("visibility:hidden");
5556
}
5657

@@ -133,4 +134,38 @@
133134
cut.Markup.ShouldContain("Please wait...");
134135
cut.Find(".spinner").ShouldNotBeNull();
135136
}
137+
138+
[Fact]
139+
public void UpdateProgress_CssClass_AppliedToDiv()
140+
{
141+
// Arrange & Act
142+
var cut = Render(
143+
@<UpdateProgress ID="styled" CssClass="progress-overlay">
144+
<ProgressTemplate>
145+
<p>Loading...</p>
146+
</ProgressTemplate>
147+
</UpdateProgress>
148+
);
149+
150+
// Assert
151+
var div = cut.Find("div#styled");
152+
div.GetAttribute("class").ShouldBe("progress-overlay");
153+
}
154+
155+
[Fact]
156+
public void UpdateProgress_NoCssClass_NoClassAttribute()
157+
{
158+
// Arrange & Act
159+
var cut = Render(
160+
@<UpdateProgress ID="unstyled">
161+
<ProgressTemplate>
162+
<p>Loading...</p>
163+
</ProgressTemplate>
164+
</UpdateProgress>
165+
);
166+
167+
// Assert
168+
var div = cut.Find("div#unstyled");
169+
div.GetAttribute("class").ShouldBeNull();
170+
}
136171
}

src/BlazorWebFormsComponents/ScriptManager.razor.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Collections.Generic;
12
using BlazorWebFormsComponents.Enums;
23
using Microsoft.AspNetCore.Components;
34

@@ -10,7 +11,7 @@ namespace BlazorWebFormsComponents
1011
public partial class ScriptManager : BaseWebFormsComponent
1112
{
1213
[Parameter]
13-
public bool EnablePartialRendering { get; set; }
14+
public bool EnablePartialRendering { get; set; } = true;
1415

1516
[Parameter]
1617
public bool EnablePageMethods { get; set; }
@@ -29,5 +30,8 @@ public partial class ScriptManager : BaseWebFormsComponent
2930

3031
[Parameter]
3132
public bool EnableScriptLocalization { get; set; }
33+
34+
[Parameter]
35+
public List<ScriptReference> Scripts { get; set; } = new();
3236
}
3337
}
Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1+
using BlazorWebFormsComponents.Enums;
2+
13
namespace BlazorWebFormsComponents
24
{
35
/// <summary>
4-
/// Represents a reference to a script file, used by ScriptManagerProxy for migration compatibility.
6+
/// Represents a reference to a script file, used by ScriptManager and ScriptManagerProxy for migration compatibility.
57
/// </summary>
68
public class ScriptReference
79
{
@@ -10,5 +12,11 @@ public class ScriptReference
1012
public string Path { get; set; }
1113

1214
public string Assembly { get; set; }
15+
16+
public ScriptMode ScriptMode { get; set; } = ScriptMode.Auto;
17+
18+
public bool NotifyScriptLoaded { get; set; } = true;
19+
20+
public string ResourceUICultures { get; set; }
1321
}
1422
}

src/BlazorWebFormsComponents/UpdateProgress.razor

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
{
55
@if (DynamicLayout)
66
{
7-
<div id="@ClientID" style="display:none;">@ProgressTemplate</div>
7+
<div id="@ClientID" class="@(string.IsNullOrEmpty(CssClass) ? null : CssClass)" style="display:none;">@ProgressTemplate</div>
88
}
99
else
1010
{
11-
<div id="@ClientID" style="visibility:hidden;">@ProgressTemplate</div>
11+
<div id="@ClientID" class="@(string.IsNullOrEmpty(CssClass) ? null : CssClass)" style="display:block;visibility:hidden;">@ProgressTemplate</div>
1212
}
1313
}

0 commit comments

Comments
 (0)