From 0b0779306b67551aee83ed2157705f1d7bfb7eed Mon Sep 17 00:00:00 2001 From: jschick04 Date: Fri, 24 Jul 2026 02:57:53 +0000 Subject: [PATCH] Fix stale highlight-tie masks and ARIA briefly rendered during async disarm (#659) The scenario histogram's highlight-tie refresh reassigned _tieHighlightFilters synchronously but replaced _baseData (and its GroupHighlightMasks) only when a background scan completed. On a disarm transition the mask-free scan did not clear _baseData synchronously, so a render in the gap mapped the old armed mask ordinals through the new (unarmed) filter list, briefly flashing stale or mis-mapped group-highlight styling and ARIA. Fixes: - Invalidate the stale GroupHighlightMasks synchronously on the rescan branch of RefreshTieFilters (covering disarm and an armed reconfigure whose eligible-list reorder remaps the old ordinals), so the render is mask-free until the correct masks republish. - Refresh the cached assertive-region bin announcement (which embeds the highlight description) on every highlight change via a shared helper, so a disarm, plan change, or color-only edit does not leave the live region announcing a stale highlight after the cursor is dismissed. - Bump the scan epoch before StartScan's zero-viewport early return and recheck the cancellation token when publishing, so a queued armed-scan publication cannot restore the just-cleared masks (a color-only disarm keeps the same predicate plan key, so the epoch bump is what supersedes it). Adds bUnit coverage for the synchronous mask clear (viewport pinned so no scan publishes), the cached-announcement refresh on both the rescan and non-rescan paths, and the zero-viewport epoch bump. --- .../LogTable/Histogram/HistogramPane.razor.cs | 43 +++-- .../LogTable/Histogram/HistogramPaneTests.cs | 159 ++++++++++++++++-- 2 files changed, 175 insertions(+), 27 deletions(-) diff --git a/src/EventLogExpert.UI/LogTable/Histogram/HistogramPane.razor.cs b/src/EventLogExpert.UI/LogTable/Histogram/HistogramPane.razor.cs index 6c66dec7..4d2f9bf1 100644 --- a/src/EventLogExpert.UI/LogTable/Histogram/HistogramPane.razor.cs +++ b/src/EventLogExpert.UI/LogTable/Histogram/HistogramPane.razor.cs @@ -369,6 +369,9 @@ private static string HighlightColorDisplayName(HighlightColor color) return joined.Length == 0 ? joined : char.ToUpperInvariant(joined[0]) + joined[1..]; } + private static bool IsCategoricalOther(HistogramData data, int group) => + group < data.Groups.Count && data.Groups[group].ColorClass == "histogram-cat-other"; + private static string? SelectActiveOriginLog(LogTableState state) { var activeTab = state.EventTables.FirstOrDefault(tab => tab.Id == state.ActiveEventLogId); @@ -534,8 +537,6 @@ private IReadOnlyList AxisLabels() return labels; } - private int BarsAreaHeight() => Math.Max(0, _plotHeightPx - AxisReservePx); - private string BarTooltip(HistogramRenderBin bin) { var start = ToDisplay(new DateTime(bin.StartTicks, DateTimeKind.Utc)); @@ -547,6 +548,8 @@ private string BarTooltip(HistogramRenderBin bin) return $"{bin.Total} {_baseData?.EventNoun ?? "events"}{GroupBreakdown(bin)}, {startText} - {endText}"; } + private int BarsAreaHeight() => Math.Max(0, _plotHeightPx - AxisReservePx); + private string BinCursorAnnouncement(HistogramRenderBin bin) { var start = ToDisplay(new DateTime(bin.StartTicks, DateTimeKind.Utc)); @@ -680,9 +683,6 @@ private string GroupHighlightText(int group) return ResolveGroupHighlight(masks[group]).Description; } - private static bool IsCategoricalOther(HistogramData data, int group) => - group < data.Groups.Count && data.Groups[group].ColorClass == "histogram-cat-other"; - private void HandleKeyDown(KeyboardEventArgs args) { if (args is { ShiftKey: true, Key: "ArrowLeft" or "ArrowRight" }) @@ -827,6 +827,14 @@ private void RecomputeSegmentHeights() if (_render is { } render && _baseData is { } data) { ComputeSegmentHeights(render, data.Groups.Count); } } + // Refresh the cached assertive-region bin readout against the current (post-filter-change) state. The cached string + // embeds the highlight description, so it must be re-derived on every highlight change (disarm, plan change, or a + // color-only edit that does not rescan) or it can keep announcing a stale highlight after the cursor is dismissed. + private void RefreshBinAnnouncement() => + _binAnnouncement = _binCursor is { } cursor && _render is { } render && cursor < render.Bins.Count + ? BinCursorAnnouncement(render.Bins[cursor]) + : string.Empty; + private void RefreshFindTicks() => _findTicks = FindMarkers.Owner == ActiveEventLogId.Value && FindMarkers.Ticks is { Count: > 0 } ticks ? [.. ticks] @@ -847,16 +855,23 @@ private void RefreshTieFilters(bool rescanOnPredicateChange) if (rescanNeeded && rescanOnPredicateChange) { + // The rescan will republish _baseData, but until it lands the current masks were resolved against the OLD + // eligible list. Invalidate them synchronously so a render in the gap does not map stale ordinals through + // the new filters (masks are non-null only when the prior state was armed). + if (_baseData is { GroupHighlightMasks: not null } data) + { + _baseData = data with { GroupHighlightMasks = null }; + } + + RefreshBinAnnouncement(); + StateHasChanged(); + StartScan(); return; } - if (_binCursor is { } cursor && _render is { } render && cursor < render.Bins.Count) - { - _binAnnouncement = BinCursorAnnouncement(render.Bins[cursor]); - } - + RefreshBinAnnouncement(); StateHasChanged(); } @@ -915,7 +930,7 @@ private async Task RunScanAsync( { await InvokeAsync(() => { - if (_disposed || epoch != _scanEpoch || !ReferenceEquals(view, ActiveView.Value)) { return; } + if (_disposed || token.IsCancellationRequested || epoch != _scanEpoch || !ReferenceEquals(view, ActiveView.Value)) { return; } if (useHighlightTie && tiePlanKey != _tiePlanKey) { return; } @@ -1030,10 +1045,14 @@ private void StartScan() _scanCts?.Dispose(); _scanCts = null; + // Bump the epoch on every supersede -- even when we bail below for a zero-size viewport -- so a prior scan whose + // UI publication is already queued is rejected by RunScanAsync's epoch guard and cannot restore stale data (for + // example, re-arm masks that a disarm just cleared). + int epoch = ++_scanEpoch; + if (_viewportWidthPx <= 0 || _plotHeightPx <= 0) { return; } IEventColumnView view = ActiveView.Value; - int epoch = ++_scanEpoch; HistogramDimension dimension = _dimension; SavedFilter[] tieHighlightFilters = _tieHighlightFilters; int tiePlanKey = _tiePlanKey; diff --git a/tests/Unit/EventLogExpert.UI.Tests/LogTable/Histogram/HistogramPaneTests.cs b/tests/Unit/EventLogExpert.UI.Tests/LogTable/Histogram/HistogramPaneTests.cs index ac4fabd8..6aaa7e3a 100644 --- a/tests/Unit/EventLogExpert.UI.Tests/LogTable/Histogram/HistogramPaneTests.cs +++ b/tests/Unit/EventLogExpert.UI.Tests/LogTable/Histogram/HistogramPaneTests.cs @@ -101,44 +101,99 @@ public HistogramPaneTests() } [Fact] - public async Task FilterRefresh_WhenOnlyHighlightColorChanges_DoesNotReadActiveViewForScan() + public async Task FilterRefresh_WhenColorOnlyEditWithNoActiveCursor_ClearsStaleBinAnnouncement() { + // Red -> blue keeps the tie armed AND the same predicate-plan key (color is excluded from the plan key), so no + // rescan occurs. The non-rescan tail must still drop the stale "Light red" announcement when no cursor is active. SavedFilter red = Filter(HighlightColor.LightRed); - SavedFilter blue = red with { Color = HighlightColor.LightBlue }; - _highlightSelector.Select(Arg.Any>()).Returns([red], [blue]); + SavedFilter blue = Filter(HighlightColor.LightBlue); + _highlightSelector.Select(Arg.Any>()).Returns([blue]); _highlightSelector.ComputePredicatePlanKey(Arg.Any>()).Returns(7); var cut = Render(); - await cut.InvokeAsync(() => cut.Instance.OnHistogramResized(500, 100)); - // The resize starts a background scan that reads ActiveView.Value twice: once synchronously at StartScan (before - // this await returns) and once in its post-scan continuation (the "did the view change mid-scan?" check). Wait for - // that second read so the continuation's read is not miscounted against the colour-only change under test (it - // otherwise races ClearReceivedCalls under load). The markup never reads ActiveView.Value, so the count is exact. - cut.WaitForAssertion(() => _ = _activeView.Received(2).Value); - _activeView.ClearReceivedCalls(); + SetPrivateField(cut, "_tieHighlightFilters", new[] { red }); + SetPrivateField(cut, "_binCursor", null); + SetPrivateField(cut, "_binAnnouncement", "2 events (1 Alpha, Light red highlight)"); + await cut.InvokeAsync(() => { }); + Assert.Contains("Light red", GetPrivateField(cut, "_binAnnouncement")); _filters.SelectedValueChanged += Raise.Event>>(_filters, ImmutableList.Create(blue)); await cut.InvokeAsync(() => { }); - _ = _activeView.DidNotReceive().Value; + Assert.Equal(string.Empty, GetPrivateField(cut, "_binAnnouncement")); } [Fact] - public async Task FilterRefresh_WhenPredicatePlanChanges_ReadsActiveViewForScan() + public async Task FilterRefresh_WhenDisarmedWithNoActiveCursor_ClearsStaleBinAnnouncement() { SavedFilter red = Filter(HighlightColor.LightRed); - SavedFilter blue = Filter(HighlightColor.LightBlue); + _highlightSelector.Select(Arg.Any>()).Returns([]); + _highlightSelector.ComputePredicatePlanKey(Arg.Any>()).Returns(7); + var cut = Render(); + + // A prior armed bin readout was cached, then the cursor was dismissed (Escape) which leaves the text behind. + SetPrivateField(cut, "_tieHighlightFilters", new[] { red }); + SetPrivateField(cut, "_binCursor", null); + SetPrivateField(cut, "_binAnnouncement", "2 events (1 Alpha, Light red highlight)"); + await cut.InvokeAsync(() => { }); + Assert.Contains("highlight", GetPrivateField(cut, "_binAnnouncement")); + + _filters.SelectedValueChanged += + Raise.Event>>(_filters, ImmutableList.Empty); + await cut.InvokeAsync(() => { }); + + Assert.Equal(string.Empty, GetPrivateField(cut, "_binAnnouncement")); + } + + [Fact] + public async Task FilterRefresh_WhenDisarmed_ClearsGroupHighlightMasksSynchronously() + { + // Arm on init, disarm on the filter change. Keep the viewport at 0 (never resize) so StartScan no-ops and cannot + // publish mask-free data on its own -- the synchronous mask clear in RefreshTieFilters is the only mutation. + SavedFilter red = Filter(HighlightColor.LightRed); + _highlightSelector.Select(Arg.Any>()).Returns([]); + _highlightSelector.ComputePredicatePlanKey(Arg.Any>()).Returns(7); + var cut = Render(); + + SetPrivateField(cut, "_tieHighlightFilters", new[] { red }); + await PublishBaseDataAsync(cut, ArmedCategoryData()); + + // Non-vacuous: the armed legend swatch renders the highlight class. + cut.WaitForAssertion(() => Assert.Equal("histogram-cat-hl", AlphaSwatchClass(cut))); + + _filters.SelectedValueChanged += + Raise.Event>>(_filters, ImmutableList.Empty); + await cut.InvokeAsync(() => { }); + + // histogram-cat-hl comes only from GroupHighlightMasks; with no scan publishing (viewport 0), its disappearance + // proves the synchronous clear ran. + cut.WaitForAssertion(() => Assert.NotEqual("histogram-cat-hl", AlphaSwatchClass(cut))); + Assert.Null(GetPrivateField(cut, "_baseData").GroupHighlightMasks); + } + + [Fact] + public async Task FilterRefresh_WhenOnlyHighlightColorChanges_DoesNotReadActiveViewForScan() + { + SavedFilter red = Filter(HighlightColor.LightRed); + SavedFilter blue = red with { Color = HighlightColor.LightBlue }; _highlightSelector.Select(Arg.Any>()).Returns([red], [blue]); - _highlightSelector.ComputePredicatePlanKey(Arg.Any>()).Returns(7, 8); + _highlightSelector.ComputePredicatePlanKey(Arg.Any>()).Returns(7); var cut = Render(); await cut.InvokeAsync(() => cut.Instance.OnHistogramResized(500, 100)); + + // The resize starts a background scan that reads ActiveView.Value twice: once synchronously at StartScan (before + // this await returns) and once in its post-scan continuation (the "did the view change mid-scan?" check). Wait for + // that second read so the continuation's read is not miscounted against the colour-only change under test (it + // otherwise races ClearReceivedCalls under load). The markup never reads ActiveView.Value, so the count is exact. + cut.WaitForAssertion(() => _ = _activeView.Received(2).Value); _activeView.ClearReceivedCalls(); _filters.SelectedValueChanged += Raise.Event>>(_filters, ImmutableList.Create(blue)); + await cut.InvokeAsync(() => { }); - _ = _activeView.Received().Value; + _ = _activeView.DidNotReceive().Value; } [Fact] @@ -166,6 +221,28 @@ public async Task FilterRefresh_WhenPredicatePlanChangesButTieStaysUnarmed_DoesN _ = _activeView.DidNotReceive().Value; } + [Fact] + public async Task FilterRefresh_WhenPredicatePlanChanges_ReadsActiveViewForScan() + { + SavedFilter red = Filter(HighlightColor.LightRed); + SavedFilter blue = Filter(HighlightColor.LightBlue); + _highlightSelector.Select(Arg.Any>()).Returns([red], [blue]); + _highlightSelector.ComputePredicatePlanKey(Arg.Any>()).Returns(7, 8); + var cut = Render(); + await cut.InvokeAsync(() => cut.Instance.OnHistogramResized(500, 100)); + + // Wait for the resize scan's two ActiveView.Value reads (sync StartScan + post-scan continuation) to settle + // before clearing, so the continuation's late read is not miscounted against the plan-change rescan under test. + cut.WaitForAssertion(() => _ = _activeView.Received(2).Value); + _activeView.ClearReceivedCalls(); + + _filters.SelectedValueChanged += + Raise.Event>>(_filters, ImmutableList.Create(blue)); + + // The plan-change rescan dispatches via InvokeAsync; wait for its ActiveView.Value read rather than checking eagerly. + cut.WaitForAssertion(() => _ = _activeView.Received().Value); + } + [Fact] public void Render_WhenDimensionRequestExists_AppliesRequestedDimensionOnMount() { @@ -300,6 +377,37 @@ public void SourceFiles_DefineExtendedHistogramPaletteAndForcedColorHatches() } } + [Fact] + public async Task StartScan_WhenViewportZero_BumpsScanEpochToSupersedeQueuedPublish() + { + // A zero-size viewport makes StartScan bail without launching a scan, but it must still bump the epoch so a prior + // scan whose UI publication is already queued is rejected and cannot restore stale (for example, armed) data. + var cut = Render(); + int before = GetPrivateField(cut, "_scanEpoch"); + + await cut.InvokeAsync(() => InvokePrivate(cut, "StartScan")); + + Assert.Equal(before + 1, GetPrivateField(cut, "_scanEpoch")); + } + + private static string? AlphaSwatchClass(IRenderedComponent cut) => + cut.FindAll(".histogram-legend-item") + .Single(button => button.TextContent == "Alpha") + .QuerySelector("rect")? + .GetAttribute("class"); + + private static HistogramData ArmedCategoryData() => + new( + [1, 1], + 2, + 1, + new DateTime(2024, 1, 1, 0, 0, 0, DateTimeKind.Utc), + new DateTime(2024, 1, 1, 1, 0, 0, DateTimeKind.Utc), + 2, + TimeSpan.FromHours(1).Ticks, + HistogramGroups.ForCategories(["alpha"], ["Alpha"], "Other"), + [1u << 1, 1u << 1]); + private static SavedFilter Filter(HighlightColor color) => new() { Color = color, IsEnabled = true, ComparisonText = "Id == 1", Compiled = null! }; @@ -313,6 +421,20 @@ private static HistogramDimension GetDimension(IRenderedComponent return Assert.IsType(field.GetValue(cut.Instance)); } + private static T GetPrivateField(IRenderedComponent cut, string name) + { + var field = typeof(HistogramPane).GetField(name, BindingFlags.Instance | BindingFlags.NonPublic); + Assert.NotNull(field); + return Assert.IsType(field.GetValue(cut.Instance)); + } + + private static void InvokePrivate(IRenderedComponent cut, string name) + { + var method = typeof(HistogramPane).GetMethod(name, BindingFlags.Instance | BindingFlags.NonPublic); + Assert.NotNull(method); + method.Invoke(cut.Instance, null); + } + private static Task PublishBaseDataAsync(IRenderedComponent cut, HistogramData data) { var field = typeof(HistogramPane).GetField( @@ -351,4 +473,11 @@ private static Task SelectDimensionAsync(IRenderedComponent cut, return cut.InvokeAsync(() => select.Instance.UpdateValue(dimension)); } + + private static void SetPrivateField(IRenderedComponent cut, string name, object? value) + { + var field = typeof(HistogramPane).GetField(name, BindingFlags.Instance | BindingFlags.NonPublic); + Assert.NotNull(field); + field.SetValue(cut.Instance, value); + } }