From 8ad02c58b99899430b2e95ae7ff579fa230cc3e3 Mon Sep 17 00:00:00 2001 From: Hitesh Kumar Date: Thu, 27 Aug 2026 13:53:58 +0530 Subject: [PATCH 1/5] Tabular: add opt-in per-cell tooltips to TableView TableView text cells render with CharacterEllipsis and no wrapping, so a value wider than its column is unreadable and there is no way for an app to surface the full text. Adds a CellToolTipRequested event: the handler receives the row's Item and the cell's Column and sets Content, which the control shows as that cell's tooltip. It is opt-in - with no handler attached the control does no per-cell work and allocates nothing - and it is resolved per cell as rows realize and recycle, so a virtualized table only pays for what is on screen. The control owns the ToolTip object. Content is the tooltip's content, not a ToolTip to attach: ToolTipService rebinds a ToolTip's owner and container on every registration, so a single instance handed back for several cells would leave them contending for one owner slot. Accessibility: the tooltip text is published as the cell's AutomationProperties.HelpText unless the cell already reports that text, so assistive technology receives the information exactly once. When Content is not a string, the handler sets ToolTipHelpText to supply the accessible equivalent - the cell wrapper the tooltip is attached to is internal, so an app cannot set HelpText on it directly. Recycling: the control tracks the tooltip it attached and the exact HelpText it published, so a recycled row never shows or announces a previous item's tooltip, including after the last handler is removed. InvalidateCellToolTips() re-resolves every realized cell for handlers attached after realization, or when the data behind the tooltips changes; it is coalesced onto the dispatcher so it is safe to call from inside a handler. Column-header and group-header tooltips are deferred. The header band is IsHitTestVisible= "False" and is scrolled programmatically from the body, so a tooltip there could never open; both surfaces land with the work that makes them interactive. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../dev/Generated/TableView.properties.cpp | 37 +++ controls/dev/Generated/TableView.properties.h | 8 + controls/dev/TableView/TableView.cpp | 56 ++++- controls/dev/TableView/TableView.h | 17 ++ controls/dev/TableView/TableView.idl | 37 +++ controls/dev/TableView/TableView.vcxitems | 2 + .../TableView/TableViewAutomationHelpers.h | 34 +++ .../TableView/TableViewCellAutomationPeer.cpp | 31 +-- .../TableViewCellToolTipRequestedEventArgs.h | 32 +++ controls/dev/TableView/TableViewRow.cpp | 209 +++++++++++++++- controls/dev/TableView/TableViewRow.h | 23 +- .../dev/TableView/TableViewToolTipHelpers.h | 228 ++++++++++++++++++ .../dev/dll-tabular/XamlMetadataProvider.cpp | 2 +- .../XamlMetadataProviderGenerated.h | 125 +++++++++- .../XamlMetadataProviderGenerated.tt | 10 + docs/api-specs/TableView/TableView-spec.md | 13 + .../TableView-functional-spec.md | 9 + 17 files changed, 828 insertions(+), 45 deletions(-) create mode 100644 controls/dev/TableView/TableViewCellToolTipRequestedEventArgs.h create mode 100644 controls/dev/TableView/TableViewToolTipHelpers.h diff --git a/controls/dev/Generated/TableView.properties.cpp b/controls/dev/Generated/TableView.properties.cpp index 4fde1aa082..af9002be27 100644 --- a/controls/dev/Generated/TableView.properties.cpp +++ b/controls/dev/Generated/TableView.properties.cpp @@ -18,6 +18,7 @@ GlobalDependencyProperty TableViewProperties::s_ColumnsProperty{ nullptr }; GlobalDependencyProperty TableViewProperties::s_DensityProperty{ nullptr }; GlobalDependencyProperty TableViewProperties::s_EmptyTemplateProperty{ nullptr }; GlobalDependencyProperty TableViewProperties::s_GridLinesVisibilityProperty{ nullptr }; +GlobalDependencyProperty TableViewProperties::s_GroupHeaderTemplateProperty{ nullptr }; GlobalDependencyProperty TableViewProperties::s_HeadersVisibilityProperty{ nullptr }; GlobalDependencyProperty TableViewProperties::s_IsReadOnlyProperty{ nullptr }; GlobalDependencyProperty TableViewProperties::s_ItemsSourceProperty{ nullptr }; @@ -29,6 +30,7 @@ GlobalDependencyProperty TableViewProperties::s_SelectionModeProperty{ nullptr } TableViewProperties::TableViewProperties() : m_beginningEditEventSource{static_cast(this)} , m_cellEditEndingEventSource{static_cast(this)} + , m_cellToolTipRequestedEventSource{static_cast(this)} , m_selectionChangedEventSource{static_cast(this)} { EnsureProperties(); @@ -91,6 +93,17 @@ void TableViewProperties::EnsureProperties() ValueHelper::BoxValueIfNecessary(winrt::TableViewGridLinesVisibility::All), winrt::PropertyChangedCallback(&OnGridLinesVisibilityPropertyChanged)); } + if (!s_GroupHeaderTemplateProperty) + { + s_GroupHeaderTemplateProperty = + InitializeDependencyProperty( + L"GroupHeaderTemplate", + winrt::name_of(), + winrt::name_of(), + false /* isAttached */, + ValueHelper::BoxedDefaultValue(), + nullptr); + } if (!s_HeadersVisibilityProperty) { s_HeadersVisibilityProperty = @@ -177,6 +190,7 @@ void TableViewProperties::ClearProperties() s_DensityProperty = nullptr; s_EmptyTemplateProperty = nullptr; s_GridLinesVisibilityProperty = nullptr; + s_GroupHeaderTemplateProperty = nullptr; s_HeadersVisibilityProperty = nullptr; s_IsReadOnlyProperty = nullptr; s_ItemsSourceProperty = nullptr; @@ -331,6 +345,19 @@ winrt::TableViewGridLinesVisibility TableViewProperties::GridLinesVisibility() return ValueHelper::CastOrUnbox(static_cast(this)->GetValue(s_GridLinesVisibilityProperty)); } +void TableViewProperties::GroupHeaderTemplate(winrt::DataTemplate const& value) +{ + [[gsl::suppress(con)]] + { + static_cast(this)->SetValue(s_GroupHeaderTemplateProperty, ValueHelper::BoxValueIfNecessary(value)); + } +} + +winrt::DataTemplate TableViewProperties::GroupHeaderTemplate() +{ + return ValueHelper::CastOrUnbox(static_cast(this)->GetValue(s_GroupHeaderTemplateProperty)); +} + void TableViewProperties::HeadersVisibility(winrt::TableViewHeadersVisibility const& value) { [[gsl::suppress(con)]] @@ -442,6 +469,16 @@ void TableViewProperties::CellEditEnding(winrt::event_token const& token) m_cellEditEndingEventSource.remove(token); } +winrt::event_token TableViewProperties::CellToolTipRequested(winrt::TypedEventHandler const& value) +{ + return m_cellToolTipRequestedEventSource.add(value); +} + +void TableViewProperties::CellToolTipRequested(winrt::event_token const& token) +{ + m_cellToolTipRequestedEventSource.remove(token); +} + winrt::event_token TableViewProperties::SelectionChanged(winrt::TypedEventHandler const& value) { return m_selectionChangedEventSource.add(value); diff --git a/controls/dev/Generated/TableView.properties.h b/controls/dev/Generated/TableView.properties.h index ca4d8cf506..7bc48590f6 100644 --- a/controls/dev/Generated/TableView.properties.h +++ b/controls/dev/Generated/TableView.properties.h @@ -24,6 +24,9 @@ class TableViewProperties void GridLinesVisibility(winrt::TableViewGridLinesVisibility const& value); winrt::TableViewGridLinesVisibility GridLinesVisibility(); + void GroupHeaderTemplate(winrt::DataTemplate const& value); + winrt::DataTemplate GroupHeaderTemplate(); + void HeadersVisibility(winrt::TableViewHeadersVisibility const& value); winrt::TableViewHeadersVisibility HeadersVisibility(); @@ -50,6 +53,7 @@ class TableViewProperties static winrt::DependencyProperty DensityProperty() { return s_DensityProperty; } static winrt::DependencyProperty EmptyTemplateProperty() { return s_EmptyTemplateProperty; } static winrt::DependencyProperty GridLinesVisibilityProperty() { return s_GridLinesVisibilityProperty; } + static winrt::DependencyProperty GroupHeaderTemplateProperty() { return s_GroupHeaderTemplateProperty; } static winrt::DependencyProperty HeadersVisibilityProperty() { return s_HeadersVisibilityProperty; } static winrt::DependencyProperty IsReadOnlyProperty() { return s_IsReadOnlyProperty; } static winrt::DependencyProperty ItemsSourceProperty() { return s_ItemsSourceProperty; } @@ -63,6 +67,7 @@ class TableViewProperties static GlobalDependencyProperty s_DensityProperty; static GlobalDependencyProperty s_EmptyTemplateProperty; static GlobalDependencyProperty s_GridLinesVisibilityProperty; + static GlobalDependencyProperty s_GroupHeaderTemplateProperty; static GlobalDependencyProperty s_HeadersVisibilityProperty; static GlobalDependencyProperty s_IsReadOnlyProperty; static GlobalDependencyProperty s_ItemsSourceProperty; @@ -75,11 +80,14 @@ class TableViewProperties void BeginningEdit(winrt::event_token const& token); winrt::event_token CellEditEnding(winrt::TypedEventHandler const& value); void CellEditEnding(winrt::event_token const& token); + winrt::event_token CellToolTipRequested(winrt::TypedEventHandler const& value); + void CellToolTipRequested(winrt::event_token const& token); winrt::event_token SelectionChanged(winrt::TypedEventHandler const& value); void SelectionChanged(winrt::event_token const& token); event_source> m_beginningEditEventSource; event_source> m_cellEditEndingEventSource; + event_source> m_cellToolTipRequestedEventSource; event_source> m_selectionChangedEventSource; static void EnsureProperties(); diff --git a/controls/dev/TableView/TableView.cpp b/controls/dev/TableView/TableView.cpp index cd9b2cec9f..b198d1a4da 100644 --- a/controls/dev/TableView/TableView.cpp +++ b/controls/dev/TableView/TableView.cpp @@ -8,6 +8,7 @@ #include "TableViewRow.h" #include "TableViewCellsPanel.h" #include "TableViewAutomationPeer.h" +#include "TableViewCellToolTipRequestedEventArgs.h" #include "RuntimeProfiler.h" #include "TVDiag.h" @@ -1130,6 +1131,59 @@ void TableView::OnRowElementPrepared( } } +winrt::com_ptr TableView::RaiseCellToolTipRequested( + const winrt::TableViewColumn& column, + const winrt::IInspectable& item) +{ + // Handler exceptions are deliberately NOT swallowed: every other event on this control lets + // them propagate so the XAML core surfaces them, and a silently missing tooltip on some rows is + // far harder to diagnose than the crash. + auto args = winrt::make_self(item, column); + m_cellToolTipRequestedEventSource(*this, *args); + return args; +} + +void TableView::InvalidateCellToolTips() +{ + // Deferred rather than raising here: this is public, so an app can call it from inside its own + // CellToolTipRequested handler, and raising synchronously would re-enter the pass and walk the + // repeater's live children while app code is mutating them. Coalesces a burst into one pass, + // matching LinedFlowLayout::InvalidateItemsInfo. + if (m_cellToolTipRefreshQueued) + { + return; + } + + auto dispatcher = DispatcherQueue(); + if (!dispatcher) + { + RefreshCellToolTipsOnRealizedRows(); + return; + } + + m_cellToolTipRefreshQueued = true; + auto weakThis = get_weak(); + if (!dispatcher.TryEnqueue([weakThis]() + { + if (auto strongThis = weakThis.get()) + { + strongThis->m_cellToolTipRefreshQueued = false; + strongThis->RefreshCellToolTipsOnRealizedRows(); + } + })) + { + m_cellToolTipRefreshQueued = false; + RefreshCellToolTipsOnRealizedRows(); + } +} + +void TableView::RefreshCellToolTipsOnRealizedRows() +{ + ForEachRealizedRow([](winrt::TableViewRow const& row) + { + winrt::get_self(row)->RefreshCellToolTips(); + }); +} void TableView::OnRowElementClearing( const winrt::ItemsRepeater& /*sender*/, const winrt::ItemsRepeaterElementClearingEventArgs& args) @@ -1375,4 +1429,4 @@ void TableView::OnTableViewUnloaded() // while detached. m_themeSettingsChangedRevoker.revoke(); m_themeSettings = nullptr; -} +} \ No newline at end of file diff --git a/controls/dev/TableView/TableView.h b/controls/dev/TableView/TableView.h index 761b56be84..35b1a83782 100644 --- a/controls/dev/TableView/TableView.h +++ b/controls/dev/TableView/TableView.h @@ -66,6 +66,8 @@ struct TableViewResourceCache double lastFrozenColumnsHorizontalOffset{ 0.0 }; }; +class TableViewCellToolTipRequestedEventArgs; + class TableView : public ReferenceTracker, public TableViewProperties @@ -139,6 +141,19 @@ class TableView : // Pin rebuilt rows immediately when leading-frozen columns are active. void PinFrozenColumnsForRow(const winrt::TableViewRow& row); + // Internal — true while at least one CellToolTipRequested handler is attached. Lets the cell + // realization path skip all per-cell tooltip work when nobody is listening. + bool HasCellToolTipHandler() const { return static_cast(m_cellToolTipRequestedEventSource); } + + // Internal — raises CellToolTipRequested for one cell and returns the args the handler filled in. + winrt::com_ptr RaiseCellToolTipRequested( + const winrt::TableViewColumn& column, + const winrt::IInspectable& item); + + // Queues a coalesced tooltip re-resolve for every realized cell (public, from the IDL). + void InvalidateCellToolTips(); + void RefreshCellToolTipsOnRealizedRows(); + // Requested by a cell panel (header/row) during measure when a realized cell's own measured width // changed (grow or shrink). Invalidates our measure synchronously so ResolveColumnWidths re-runs in // the same layout tick (no deferral -> no one-frame lag). Bridges the body ScrollViewer, which @@ -556,6 +571,8 @@ class TableView : // Set while a coalesced RebuildHeaders is pending on the dispatcher; collapses a burst of column // changes into one rebuild. UI-thread only (all column callbacks arrive on the UI thread). bool m_rebuildHeadersQueued{ false }; + // A coalesced cell-tooltip re-resolve is pending on the dispatcher (InvalidateCellToolTips). + bool m_cellToolTipRefreshQueued{ false }; // Per-instance resource cache; replaces the former process-global map keyed by `this`. TableViewResourceCache m_resourceCache{}; diff --git a/controls/dev/TableView/TableView.idl b/controls/dev/TableView/TableView.idl index 5ae18271fa..dfec290e13 100644 --- a/controls/dev/TableView/TableView.idl +++ b/controls/dev/TableView/TableView.idl @@ -185,6 +185,29 @@ runtimeclass TableViewCellEditEndingEventArgs Boolean Cancel; }; +// Raised while a cell is being realized so the app can supply that cell's tooltip. +// Opt-in: with no handler attached the control does no per-cell work. +[MUX_PREVIEW] +[webhosthidden] +runtimeclass TableViewCellToolTipRequestedEventArgs +{ + // The data item of the row the cell belongs to. + Object Item { get; }; + + // The column of the cell being realized. Always non-null. + MU_XC_NAMESPACE.TableViewColumn Column { get; }; + + // Set by the handler to the tooltip content. Any non-null value is honoured, including a + // configured ToolTip (the way to override placement) or non-string content such as a panel; + // leaving it null (the default) means "no tooltip for this cell". + Object Content { get; set; }; + + // Accessible text for this cell's tooltip, surfaced as the cell's UIA HelpText. Set this when + // Content is not a string -- non-string content cannot be read by assistive technology, and + // the cell wrapper is not reachable from the app. String Content is used automatically. + String ToolTipHelpText { get; set; }; +}; + [MUX_PREVIEW] [webhosthidden] unsealed runtimeclass TableViewTextColumn : TableViewColumn @@ -445,6 +468,20 @@ unsealed runtimeclass TableView : Microsoft.UI.Xaml.Controls.Control event Windows.Foundation.TypedEventHandler SelectionChanged; + // ----- Tooltips ----- + // Raised when a cell's tooltip is resolved -- on realization, on cell rebuild, and on + // InvalidateCellToolTips. May be raised repeatedly for the same cell and item, so handlers + // should be cheap and free of side effects. Handlers set args.Content; any non-null content + // produces the tooltip. A tooltip set inside a cell's own content template opens over that + // content, and the control's cell tooltip covers the rest of the cell. Not raised when no + // handler is attached; handlers added or removed after rows are realized take effect as those + // rows are re-realized, or on the next InvalidateCellToolTips. + event Windows.Foundation.TypedEventHandler CellToolTipRequested; + + // Re-raises CellToolTipRequested for every realized cell. Call after attaching a handler to a + // TableView whose rows are already realized, or when the data behind the tooltips changes. + void InvalidateCellToolTips(); + static Microsoft.UI.Xaml.DependencyProperty ItemsSourceProperty { get; }; static Microsoft.UI.Xaml.DependencyProperty ColumnsProperty { get; }; static Microsoft.UI.Xaml.DependencyProperty HeadersVisibilityProperty { get; }; diff --git a/controls/dev/TableView/TableView.vcxitems b/controls/dev/TableView/TableView.vcxitems index 8a88c1cf39..506324ea5c 100644 --- a/controls/dev/TableView/TableView.vcxitems +++ b/controls/dev/TableView/TableView.vcxitems @@ -41,6 +41,8 @@ + + diff --git a/controls/dev/TableView/TableViewAutomationHelpers.h b/controls/dev/TableView/TableViewAutomationHelpers.h index 617b1803ef..e19bc2819f 100644 --- a/controls/dev/TableView/TableViewAutomationHelpers.h +++ b/controls/dev/TableView/TableViewAutomationHelpers.h @@ -50,3 +50,37 @@ inline std::optional TryGetColumnHeaderString(winrt::TableViewCo } return std::nullopt; } + +// The text a realized cell reports to UIA: the generated TextBlock for a text column, otherwise the +// standard UIA name of the column-generated content. Shared by TableViewCellAutomationPeer (which +// composes it into the cell name) and the cell tooltip pass (which uses it to avoid publishing +// HelpText that would make Narrator read the same string twice), so the two cannot drift. +inline winrt::hstring GetCellValueTextFromWrapper(winrt::FrameworkElement const& cellWrapper) +{ + if (!cellWrapper) + { + return {}; + } + + winrt::FrameworkElement content{ nullptr }; + if (auto const border = cellWrapper.try_as()) + { + content = border.Child().try_as(); + } + if (!content) + { + content = cellWrapper; + } + + if (auto const textBlock = content.try_as()) + { + return textBlock.Text(); + } + + if (auto const peer = winrt::FrameworkElementAutomationPeer::CreatePeerForElement(content)) + { + return peer.GetName(); + } + + return {}; +} diff --git a/controls/dev/TableView/TableViewCellAutomationPeer.cpp b/controls/dev/TableView/TableViewCellAutomationPeer.cpp index f2ecdb278c..2a78f3d2a7 100644 --- a/controls/dev/TableView/TableViewCellAutomationPeer.cpp +++ b/controls/dev/TableView/TableViewCellAutomationPeer.cpp @@ -97,36 +97,7 @@ winrt::hstring TableViewCellAutomationPeer::GetColumnHeaderText() winrt::hstring TableViewCellAutomationPeer::GetCellValueText() { - auto const cell = Owner().try_as(); - if (!cell) - { - return {}; - } - - // The cell wrapper's child is the column-generated content. - winrt::FrameworkElement content{ nullptr }; - if (auto const border = cell.try_as()) - { - content = border.Child().try_as(); - } - if (!content) - { - content = cell; - } - - // Common text-column case: read the generated TextBlock. - if (auto const textBlock = content.try_as()) - { - return textBlock.Text(); - } - - // Template content uses the standard UIA name computation. - if (auto const peer = winrt::FrameworkElementAutomationPeer::CreatePeerForElement(content)) - { - return peer.GetName(); - } - - return {}; + return GetCellValueTextFromWrapper(Owner().try_as()); } int32_t TableViewCellAutomationPeer::GetRowIndex() diff --git a/controls/dev/TableView/TableViewCellToolTipRequestedEventArgs.h b/controls/dev/TableView/TableViewCellToolTipRequestedEventArgs.h new file mode 100644 index 0000000000..3e61cb1536 --- /dev/null +++ b/controls/dev/TableView/TableViewCellToolTipRequestedEventArgs.h @@ -0,0 +1,32 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. See LICENSE in the project root for license information. + +#pragma once + +#include "TableViewCellToolTipRequestedEventArgs.g.h" + +class TableViewCellToolTipRequestedEventArgs : + public winrt::implementation::TableViewCellToolTipRequestedEventArgsT +{ +public: + TableViewCellToolTipRequestedEventArgs( + winrt::IInspectable const& item, + winrt::TableViewColumn const& column) + : m_column(column) + , m_item(item) + { + } + + winrt::IInspectable Item() { return m_item; } + winrt::TableViewColumn Column() { return m_column; } + winrt::IInspectable Content() { return m_content; } + void Content(winrt::IInspectable const& value) { m_content = value; } + winrt::hstring ToolTipHelpText() { return m_toolTipHelpText; } + void ToolTipHelpText(winrt::hstring const& value) { m_toolTipHelpText = value; } + +private: + winrt::TableViewColumn m_column{ nullptr }; + winrt::IInspectable m_item{ nullptr }; + winrt::IInspectable m_content{ nullptr }; + winrt::hstring m_toolTipHelpText{}; +}; diff --git a/controls/dev/TableView/TableViewRow.cpp b/controls/dev/TableView/TableViewRow.cpp index 1778b0f363..1140a4c461 100644 --- a/controls/dev/TableView/TableViewRow.cpp +++ b/controls/dev/TableView/TableViewRow.cpp @@ -4,6 +4,9 @@ #include "pch.h" #include "common.h" #include "TableViewRow.h" +#include "TableViewAutomationHelpers.h" +#include "TableViewToolTipHelpers.h" +#include "TableViewCellToolTipRequestedEventArgs.h" #include "TableView.h" #include "TableViewColumn.h" #include "TableViewCellsPanel.h" @@ -284,13 +287,9 @@ void TableViewRow::OnDataContextChanged( // On recycle ItemsRepeater updates the row's DataContext; cells pick up the new item reactively // via inheritance (they are not restamped). RebuildCells is still called so index-dependent visuals // (alternating-row banding, frozen pinning) refresh for the new position. Guard against re-entry - // from child DataContext propagation. - if (m_isRebuildingCells) - { - return; - } - RebuildCells(); -} + // Guarded by RebuildCells, which owns the re-entry policy for both rebuilds and the tooltip + // pass; a bare guard here would drop the request instead of recording it. + RebuildCells();} void TableViewRow::OnColumnsVectorChanged( const winrt::IObservableVector& /*sender*/, @@ -545,6 +544,55 @@ void TableViewRow::InvalidateCells() } void TableViewRow::RebuildCells() +{ + // The tooltip pass reaches app code, so it must not run under the re-entry guard: a handler + // that replaces Columns re-enters here, and a swallowed rebuild would leave the row empty with + // nothing queued to retry it. + auto strongThis = get_strong(); + + if (m_isRebuildingCells || m_isRefreshingCellToolTips) + { + RebuildCellsCore(); + return; + } + + // One rebuild, plus replays for app code that mutates the row from a tooltip handler; the + // tooltip pass itself can queue one, so it runs inside the drain. Beyond the cap the app is + // livelocking the UI thread, so the request is dropped with a diagnostic rather than spun on. + constexpr int c_maxRebuildDrainPasses = 3; + for (int pass = 0; pass < c_maxRebuildDrainPasses; ++pass) + { + m_rebuildCellsPending = false; + RebuildCellsCore(); + + // Contained here rather than at the raise: this runs from framework callbacks + // (OnApplyTemplate, OnDataContextChanged, the repeater's measure pass), where letting an app + // handler's exception cross back into XAML is a fail-fast rather than a recoverable error. + try + { + RefreshCellToolTips(); + } + catch (...) + { + TVDiag::LogRetailF(L"[TableViewRow] CellToolTipRequested handler threw (HRESULT 0x%08X). Continuing.", + static_cast(winrt::to_hresult())); + } + + if (!m_rebuildCellsPending) + { + break; + } + } + + if (m_rebuildCellsPending) + { + TVDiag::LogRetailF(L"[TableViewRow] Dropping a pending cell rebuild after %d drain passes.", + c_maxRebuildDrainPasses); + m_rebuildCellsPending = false; + } +} + +void TableViewRow::RebuildCellsCore() { auto host = m_cellsHost.get(); if (!host) @@ -552,13 +600,16 @@ void TableViewRow::RebuildCells() return; } - // Re-entry guard. See OnDataContextChanged. - if (m_isRebuildingCells) + // Re-entry guard. See OnDataContextChanged. A request that arrives while we are rebuilding, or + // while the tooltip pass is calling into app code, is recorded rather than dropped: rebuilding + // mid-pass would detach the very wrappers that pass is still applying tooltips to. + if (m_isRebuildingCells || m_isRefreshingCellToolTips) { + m_rebuildCellsPending = true; return; } m_isRebuildingCells = true; - // Synchronous RAII guard: captures this only until RebuildCells returns. + // Synchronous RAII guard: captures this only until RebuildCellsCore returns. auto rebuildGuard = wil::scope_exit([this]() { m_isRebuildingCells = false; }); auto owner = GetOwningTableView(); @@ -796,6 +847,140 @@ void TableViewRow::RebuildCells() RefreshRowBackground(); } +void TableViewRow::RefreshCellToolTips() +{ + // The guard lives here, not at the caller: this is the function that calls into app code, and it + // has more than one entry point (cell rebuild, and TableView::InvalidateCellToolTips). A handler + // that triggers either one re-enters, and without this it would recurse until the stack died. + if (m_isRefreshingCellToolTips) + { + return; + } + + auto const host = m_cellsHost.get(); + if (!host) + { + return; + } + + auto strongThis = get_strong(); + m_isRefreshingCellToolTips = true; + auto refreshGuard = wil::scope_exit([this]() { m_isRefreshingCellToolTips = false; }); + + auto const owner = GetOwningTableView(); + // Recycled out: the row keeps its cells for the next prepare, but an owned tooltip can hold + // app-supplied content, so release it rather than pinning it in the pool. + if (!owner) + { + ClearOwnedCellToolTips(host); + return; + } + + auto const ownerImpl = winrt::get_self(owner); + + // Opt-in: with no handler this costs one event_source test per row and allocates nothing -- + // unless a previous pass left tooltips behind, in which case they must be retracted or a + // recycled cell would keep showing the previous item's text. + if (!ownerImpl->HasCellToolTipHandler()) + { + if (m_hasOwnedCellToolTips) + { + ClearOwnedCellToolTips(host); + } + return; + } + + // The raise reaches app code, which may mutate Columns or the cell children, so the targets are + // snapshotted first rather than walking the live collection. The scratch vector is moved into a + // local so a nested pass cannot clear it while this one iterates; the capacity is handed back. + auto targets = std::move(m_toolTipTargets); + targets.clear(); + auto const children = host.Children(); + const uint32_t count = children.Size(); + targets.reserve(count); + auto const editingWrapper = m_editingCellWrapper.get(); + for (uint32_t i = 0; i < count; ++i) + { + // Target the wrapper, not the display element: the wrapper fills the cell, so the whole + // cell is the hover target, and it is also the UIA cell node. + auto const cellWrapper = children.GetAt(i).try_as(); + if (!cellWrapper) + { + continue; + } + + // An open editor owns its cell: a tooltip over a live text box is noise. Retract rather + // than skip -- skipping would leave an enabled tooltip behind that no later pass clears. + if (cellWrapper == editingWrapper) + { + TableViewDetails::ClearOwnedToolTip(cellWrapper); + continue; + } + + if (auto const column = GetCellOwningColumn(cellWrapper)) + { + targets.emplace_back(column, cellWrapper); + } + } + + // Commit the ownership flag and hand the buffer back on every exit, including the exception path + // -- a handler is allowed to throw, and a row that has tooltips must know to clear them later. + bool anyOwned = false; + auto passGuard = wil::scope_exit([&]() + { + m_hasOwnedCellToolTips = anyOwned; + targets.clear(); + m_toolTipTargets = std::move(targets); + }); + + auto const dataItem = ownerImpl->UnwrapEditingDataItem(DataContext()); + for (auto const& target : targets) + { + auto const args = ownerImpl->RaiseCellToolTipRequested(target.first, dataItem); + auto const content = args->Content(); + + try + { + // Contained separately from the raise -- a handler's own exception propagates, but + // app-supplied content may be an already-parented element, which throws on assignment. + // The UIA text is only needed to suppress a duplicate announcement, so it is resolved + // lazily rather than for every cell including the ones with no tooltip. + const bool owned = content + ? TableViewDetails::SetOwnedToolTip( + target.second, + content, + GetCellValueTextFromWrapper(target.second), + args->ToolTipHelpText(), + winrt::PlacementMode::Mouse) + : (TableViewDetails::ClearOwnedToolTip(target.second), false); + + anyOwned = anyOwned || owned; + } + catch (...) + { + // Assume the element still holds something: the row must revisit it rather than skip it. + anyOwned = true; + TVDiag::LogRetailF(L"[TableViewRow] Applying CellToolTipRequested content failed (HRESULT 0x%08X).", + static_cast(winrt::to_hresult())); + } + } +} + +void TableViewRow::ClearOwnedCellToolTips(const winrt::Panel& host) +{ + auto const children = host.Children(); + const uint32_t count = children.Size(); + for (uint32_t i = 0; i < count; ++i) + { + if (auto const cellWrapper = children.GetAt(i).try_as()) + { + TableViewDetails::ClearOwnedToolTip(cellWrapper); + } + } + + m_hasOwnedCellToolTips = false; +} + // Installs a generated display element as a cell's content, including the ContentPresenter wiring a // template column needs. Shared by the cell rebuild and by the post-commit refresh, because // GenerateElement alone is NOT a complete cell - forgetting the second half leaves a template @@ -1076,6 +1261,10 @@ void TableViewRow::EndCellEdit(winrt::TableViewEditAction action) m_editingCellWrapper.set(nullptr); m_editingElement.set(nullptr); m_editingDisplayElement.set(nullptr); + + // The tooltip pass retracts the editing cell's tooltip while the editor is open; nothing else + // brings it back, so re-resolve now that the cell is a display cell again. + RefreshCellToolTips(); } void TableViewRow::AbandonCellEdit() diff --git a/controls/dev/TableView/TableViewRow.h b/controls/dev/TableView/TableViewRow.h index 9c19e666ad..42ab23267c 100644 --- a/controls/dev/TableView/TableViewRow.h +++ b/controls/dev/TableView/TableViewRow.h @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. See LICENSE in the project root for license information. #pragma once @@ -9,6 +9,9 @@ #include "TableViewRow.g.h" #include "TableViewRow.properties.h" +#include +#include + class TableViewRow : public ReferenceTracker, public TableViewRowProperties @@ -55,6 +58,11 @@ class TableViewRow : void RefreshDensity(); // Rebuild realized cells when column content changes at runtime. void RefreshCells(); + + // Internal: re-resolves this row's cell tooltips against its current data item. Raised outside + // the cell-rebuild re-entry guard because it calls into app code. + void RefreshCellToolTips(); + // Apply a column's current visibility to this row's matching cell (the row owns its cells, // so TableView asks the row instead of reaching into the row's cell panel). The visibility is // passed in (snapshotted once by the caller) so header and all rows apply the same value. @@ -128,6 +136,9 @@ class TableViewRow : const winrt::Microsoft::UI::Xaml::DependencyPropertyChangedEventArgs& args); void RebuildCells(); + // The guarded body. RebuildCells wraps it so the tooltip pass runs with the guard released. + void RebuildCellsCore(); + void ClearOwnedCellToolTips(const winrt::Panel& host); // Coalesces a burst of Columns-collection changes into a single cell rebuild on the next // dispatcher tick (each realized row observes Columns, so N bulk edits would otherwise cost N @@ -154,6 +165,16 @@ class TableViewRow : // Prevent DataContextChanged re-entry while RebuildCells updates child DCs. bool m_isRebuildingCells{ false }; + // Set while the tooltip pass is running, so a nested RebuildCells from a handler rebuilds + // without starting another raise. + bool m_isRefreshingCellToolTips{ false }; + // A rebuild requested while the guard was held; replayed once the guard drops. + bool m_rebuildCellsPending{ false }; + // Whether any cell on this row currently carries a control-created tooltip. Lets the pass stay + // free when the feature is unused, while still retracting tooltips after the last handler goes. + bool m_hasOwnedCellToolTips{ false }; + // Reused per pass so the virtualized path does not allocate on every recycle. + std::vector> m_toolTipTargets{}; // Set while a coalesced RebuildCells is pending on the dispatcher (Columns-vector-changed burst). bool m_rebuildCellsQueued{ false }; diff --git a/controls/dev/TableView/TableViewToolTipHelpers.h b/controls/dev/TableView/TableViewToolTipHelpers.h new file mode 100644 index 0000000000..5cf29f59b7 --- /dev/null +++ b/controls/dev/TableView/TableViewToolTipHelpers.h @@ -0,0 +1,228 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. See LICENSE in the project root for license information. + +#pragma once + +#include "pch.h" +#include "common.h" +#include "GlobalDependencyProperty.h" +#include "CppWinRTHelpers.h" + +#include + +// Tooltip plumbing for the cell realization path. Kept out of TableView so the helpers stay element +// operations with no control state, next to TableViewAutomationHelpers.h / TableViewGroupingHelpers.h. +namespace TableViewDetails +{ + // Everything the control needs to know about the tooltip it put on an element: the object it + // attached, and the exact HelpText it published for it. Both facts have to be recorded rather + // than re-derived -- the published text is not always the tooltip's content (an app can supply + // its own accessible text for non-string content), so deriving it would strand stale HelpText + // on a recycled cell. + struct CellToolTipRecord : winrt::implements + { + winrt::ToolTip ToolTip{ nullptr }; + winrt::hstring PublishedHelpText{}; + }; + + // Deliberately not in the IDL, so an app cannot reach or overwrite the record -- unlike the + // ToolTip's public Tag, which an app can reach through ToolTipService::GetToolTip. + // + // GlobalDependencyProperty rather than a winrt::DependencyProperty static: it must not run a + // destructor at DLL unload, and it has to be re-registerable after ClearProperties. + inline GlobalDependencyProperty s_cellToolTipRecordProperty{ nullptr }; + + inline winrt::DependencyProperty EnsureCellToolTipRecordProperty() + { + if (!s_cellToolTipRecordProperty) + { + s_cellToolTipRecordProperty = InitializeDependencyProperty( + L"TableViewCellToolTipRecord", + winrt::name_of(), + winrt::name_of(), + true /* isAttached */, + nullptr /* defaultValue */, + nullptr /* propertyChangedCallback */); + } + + return s_cellToolTipRecordProperty; + } + + // Paired with EnsureCellToolTipRecordProperty, and called from the same teardown as the + // generated TableViewProperties::ClearProperties so a XAML re-init re-registers against the new + // core rather than handing out a registration from the dead one. + inline void ClearCellToolTipProperties() + { + s_cellToolTipRecordProperty = nullptr; + } + + inline winrt::com_ptr GetRecord(const winrt::FrameworkElement& element) + { + if (!element || !s_cellToolTipRecordProperty) + { + return nullptr; + } + + return element.GetValue(s_cellToolTipRecordProperty).try_as(); + } + + inline void ForgetRecord(const winrt::FrameworkElement& element) + { + if (s_cellToolTipRecordProperty) + { + element.ClearValue(s_cellToolTipRecordProperty); + } + } + + inline std::optional TryGetString(const winrt::IInspectable& value) + { + if (auto const propertyValue = value ? value.try_as() : nullptr; + propertyValue && propertyValue.Type() == winrt::PropertyType::String) + { + return propertyValue.GetString(); + } + + return std::nullopt; + } + + // Retracts the HelpText this control published for the element, if it is still the value there. + inline void RetractPublishedHelpText(const winrt::FrameworkElement& element, CellToolTipRecord& record) + { + if (!record.PublishedHelpText.empty() && + winrt::AutomationProperties::GetHelpText(element) == record.PublishedHelpText) + { + winrt::AutomationProperties::SetHelpText(element, winrt::hstring{}); + } + + record.PublishedHelpText = {}; + } + + // Retracts the tooltip this control put on the element. The ToolTip object stays attached and is + // neutralized rather than detached, so a recycled cell reuses it instead of allocating a fresh + // Control per cell per scroll -- matching TabViewItem and NavigationViewItem. + inline void ClearOwnedToolTip(const winrt::FrameworkElement& element) + { + auto const record = GetRecord(element); + if (!record) + { + return; + } + + RetractPublishedHelpText(element, *record); + + // Still ours only if the object we recorded is the one currently attached; otherwise the app + // replaced or removed it and we must not touch what is there now. + if (record->ToolTip && record->ToolTip == winrt::ToolTipService::GetToolTip(element).try_as()) + { + // Disable before dropping the content: clearing the content of a tooltip that is + // currently open removes a live popup's child, which is the shape behind the reentrant + // CPopup::RemoveChild teardown crash. + record->ToolTip.IsEnabled(false); + if (record->ToolTip.IsOpen()) + { + record->ToolTip.IsOpen(false); + } + record->ToolTip.Content(nullptr); + } + else + { + ForgetRecord(element); + } + } + + // Applies app-supplied tooltip content to a control-created cell wrapper, and returns whether the + // element is left carrying a control-owned tooltip. Never touches a tooltip the app set itself. + // + // `existingUiaText` is what the element already reports to UIA; `helpTextOverride` is the app's + // accessible text for content UIA cannot read. The resulting text is published as HelpText unless + // the element already reports it, so assistive technology gets the information exactly once. + inline bool SetOwnedToolTip( + const winrt::FrameworkElement& element, + const winrt::IInspectable& content, + const winrt::hstring& existingUiaText, + const winrt::hstring& helpTextOverride, + winrt::PlacementMode placement) + { + if (!element) + { + return false; + } + + auto record = GetRecord(element); + auto const existing = winrt::ToolTipService::GetToolTip(element).try_as(); + auto const owned = (record && record->ToolTip && record->ToolTip == existing) ? record->ToolTip : nullptr; + + // The app set or replaced the tooltip on this element: retract what we published and stop + // treating the element as ours. + if (existing && !owned) + { + if (record) + { + RetractPublishedHelpText(element, *record); + ForgetRecord(element); + } + return false; + } + + auto const text = TryGetString(content); + // An empty string is never content -- it would pop an empty tooltip. + if (!content || (text && text->empty())) + { + ClearOwnedToolTip(element); + return false; + } + + EnsureCellToolTipRecordProperty(); + if (!record) + { + record = winrt::make_self(); + element.SetValue(s_cellToolTipRecordProperty, *record); + } + + // Retract the previous pairing before the content changes, or HelpText published for the + // previous item would outlive it. + RetractPublishedHelpText(element, *record); + + if (owned) + { + // Neutralize first so a throwing assignment cannot leave the previous item's content + // enabled on a recycled cell, and so the content is never swapped under an open popup. + owned.IsEnabled(false); + if (owned.IsOpen()) + { + owned.IsOpen(false); + } + owned.Content(nullptr); + owned.Content(content); + owned.Placement(placement); + owned.IsEnabled(true); + } + else + { + winrt::ToolTip toolTip; + toolTip.Content(content); + toolTip.Placement(placement); + + // Record before attaching. The other order leaves an attached tooltip with no record if + // the write throws, which the next pass would read as app-set and never clear; this way a + // throw leaves a record for a tooltip that is not attached, which reads as "not ours". + record->ToolTip = toolTip; + winrt::ToolTipService::SetToolTip(element, toolTip); + } + + // Publish the accessible text unless the element already reports it, so Narrator does not read + // the same string twice. Only text this control published is ever overwritten. + auto const helpText = !helpTextOverride.empty() ? helpTextOverride : (text ? *text : winrt::hstring{}); + if (!helpText.empty() && helpText != existingUiaText) + { + auto const current = winrt::AutomationProperties::GetHelpText(element); + if (current.empty()) + { + winrt::AutomationProperties::SetHelpText(element, helpText); + record->PublishedHelpText = helpText; + } + } + + return true; + } +} diff --git a/controls/dev/dll-tabular/XamlMetadataProvider.cpp b/controls/dev/dll-tabular/XamlMetadataProvider.cpp index 51322afe0a..c6f630d16f 100644 --- a/controls/dev/dll-tabular/XamlMetadataProvider.cpp +++ b/controls/dev/dll-tabular/XamlMetadataProvider.cpp @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. See LICENSE in the project root for license information. #include "pch.h" diff --git a/controls/dev/dll-tabular/XamlMetadataProviderGenerated.h b/controls/dev/dll-tabular/XamlMetadataProviderGenerated.h index ae763b57c9..a0532c42c8 100644 --- a/controls/dev/dll-tabular/XamlMetadataProviderGenerated.h +++ b/controls/dev/dll-tabular/XamlMetadataProviderGenerated.h @@ -101,6 +101,7 @@ Entry c_typeEntries[] = xamlType.AddDPMember(L"Density", L"Microsoft.UI.Xaml.Controls.Tabular.TableViewDensity", statics.DensityProperty(), false /* isContent */); xamlType.AddDPMember(L"EmptyTemplate", L"Microsoft.UI.Xaml.DataTemplate", statics.EmptyTemplateProperty(), false /* isContent */); xamlType.AddDPMember(L"GridLinesVisibility", L"Microsoft.UI.Xaml.Controls.Tabular.TableViewGridLinesVisibility", statics.GridLinesVisibilityProperty(), false /* isContent */); + xamlType.AddDPMember(L"GroupHeaderTemplate", L"Microsoft.UI.Xaml.DataTemplate", statics.GroupHeaderTemplateProperty(), false /* isContent */); xamlType.AddDPMember(L"HeadersVisibility", L"Microsoft.UI.Xaml.Controls.Tabular.TableViewHeadersVisibility", statics.HeadersVisibilityProperty(), false /* isContent */); xamlType.AddDPMember(L"IsReadOnly", L"Boolean", statics.IsReadOnlyProperty(), false /* isContent */); xamlType.AddDPMember(L"ItemsSource", L"Object", statics.ItemsSourceProperty(), false /* isContent */); @@ -263,6 +264,110 @@ Entry c_typeEntries[] = return xamlType; } }, + { + /* Arg1 TypeName */ + L"Microsoft.UI.Xaml.Controls.Tabular.TableViewGroupHeader", + /* Arg2 CreateXamlTypeCallback */ + []() + { + auto xamlType = winrt::make_self( + /* Arg 1 - TypeName */ + (PCWSTR)L"Microsoft.UI.Xaml.Controls.Tabular.TableViewGroupHeader", + /* Arg 2 - BaseTypeName */ + (PCWSTR)L"Microsoft.UI.Xaml.Controls.ContentControl", + /* Arg 3 - Activator func */ + (std::function)[](){ return ActivateInstanceWithFactory(L"Microsoft.UI.Xaml.Controls.Tabular.TableViewGroupHeader"); }, + /* Arg 4 - Populate properties func */ + (std::function)[](XamlTypeBase& xamlType) + { + winrt::ITableViewGroupHeaderStatics statics = GetFactory(L"Microsoft.UI.Xaml.Controls.Tabular.TableViewGroupHeader"); + { + xamlType.AddDPMember(L"IsExpandable", L"Boolean", statics.IsExpandableProperty(), false /* isContent */); + xamlType.AddDPMember(L"IsExpanded", L"Boolean", statics.IsExpandedProperty(), false /* isContent */); + } + + }); + + return static_cast(*xamlType); + } + }, + { + /* Arg1 TypeName */ + L"Microsoft.UI.Xaml.Controls.Tabular.TableViewGroupInfo", + /* Arg2 CreateXamlTypeCallback */ + []() + { + auto xamlType = winrt::make_self( + /* Arg 1 - TypeName */ + (PCWSTR)L"Microsoft.UI.Xaml.Controls.Tabular.TableViewGroupInfo", + /* Arg 2 - BaseTypeName */ + (PCWSTR)L"Object", + /* Arg 3 - Activator func */ + nullptr, + /* Arg 4 - Populate properties func */ + (std::function)[](XamlTypeBase& xamlType) + { + xamlType.AddMember( + L"IsExpandable", /* propertyName */ + L"Boolean", /* propertyType */ + [](winrt::IInspectable instance) { return box_value(instance.as().IsExpandable()); }, + nullptr, /* setter */ + false, /* isContent */ + false, /* isDependencyProperty */ + false /* isAttachable */); + xamlType.AddMember( + L"IsExpanded", /* propertyName */ + L"Boolean", /* propertyType */ + [](winrt::IInspectable instance) { return box_value(instance.as().IsExpanded()); }, + nullptr, /* setter */ + false, /* isContent */ + false, /* isDependencyProperty */ + false /* isAttachable */); + xamlType.AddMember( + L"ItemCount", /* propertyName */ + L"Int32", /* propertyType */ + [](winrt::IInspectable instance) { return box_value(instance.as().ItemCount()); }, + nullptr, /* setter */ + false, /* isContent */ + false, /* isDependencyProperty */ + false /* isAttachable */); + xamlType.AddMember( + L"ItemCountText", /* propertyName */ + L"String", /* propertyType */ + [](winrt::IInspectable instance) { return box_value(instance.as().ItemCountText()); }, + nullptr, /* setter */ + false, /* isContent */ + false, /* isDependencyProperty */ + false /* isAttachable */); + xamlType.AddMember( + L"Key", /* propertyName */ + L"Object", /* propertyType */ + [](winrt::IInspectable instance) { return instance.as().Key(); }, + nullptr, /* setter */ + false, /* isContent */ + false, /* isDependencyProperty */ + false /* isAttachable */); + xamlType.AddMember( + L"KeyText", /* propertyName */ + L"String", /* propertyType */ + [](winrt::IInspectable instance) { return box_value(instance.as().KeyText()); }, + nullptr, /* setter */ + false, /* isContent */ + false, /* isDependencyProperty */ + false /* isAttachable */); + xamlType.AddMember( + L"Level", /* propertyName */ + L"Int32", /* propertyType */ + [](winrt::IInspectable instance) { return box_value(instance.as().Level()); }, + nullptr, /* setter */ + false, /* isContent */ + false, /* isDependencyProperty */ + false /* isAttachable */); + }); + + return static_cast(*xamlType); + } + }, { /* Arg1 TypeName */ L"Microsoft.UI.Xaml.Controls.Tabular.TableViewHeadersVisibility", @@ -607,6 +712,12 @@ Entry c_typeEntries[] = /* Arg2 CreateXamlTypeCallback */ []() { return winrt::make((PCWSTR)L"Double"); } }, + { + /* Arg1 TypeName */ + L"Int32", + /* Arg2 CreateXamlTypeCallback */ + []() { return winrt::make((PCWSTR)L"Int32"); } + }, { /* Arg1 TypeName */ L"Microsoft.UI.Private.Controls.SortIndicatorDirection", @@ -630,9 +741,9 @@ Entry c_typeEntries[] = }, { /* Arg1 TypeName */ - L"Int32", + L"Microsoft.UI.Xaml.Controls.ContentControl", /* Arg2 CreateXamlTypeCallback */ - []() { return winrt::make((PCWSTR)L"Int32"); } + []() { return winrt::make((PCWSTR)L"Microsoft.UI.Xaml.Controls.ContentControl"); } }, { /* Arg1 TypeName */ @@ -700,6 +811,12 @@ Entry c_typeEntries[] = /* Arg2 CreateXamlTypeCallback */ []() { return winrt::make((PCWSTR)L"Object"); } }, + { + /* Arg1 TypeName */ + L"String", + /* Arg2 CreateXamlTypeCallback */ + []() { return winrt::make((PCWSTR)L"String"); } + }, { /* Arg1 TypeName */ L"ValueType", @@ -738,8 +855,10 @@ std::wstring_view c_knownNamespacePrefixes[] = #include "SortIndicator.properties.h" #include "TableView.properties.h" #include "TableViewColumn.properties.h" +#include "TableViewGroupHeader.properties.h" #include "TableViewRow.properties.h" #include "TableViewTemplateColumn.properties.h" +#include "TableViewToolTipHelpers.h" namespace { @@ -748,8 +867,10 @@ void ClearTypeProperties() SortIndicatorProperties::ClearProperties(); TableViewProperties::ClearProperties(); TableViewColumnProperties::ClearProperties(); + TableViewGroupHeaderProperties::ClearProperties(); TableViewRowProperties::ClearProperties(); TableViewTemplateColumnProperties::ClearProperties(); + TableViewDetails::ClearCellToolTipProperties(); } } diff --git a/controls/dev/dll-tabular/XamlMetadataProviderGenerated.tt b/controls/dev/dll-tabular/XamlMetadataProviderGenerated.tt index 3d1ccec636..460a594a86 100644 --- a/controls/dev/dll-tabular/XamlMetadataProviderGenerated.tt +++ b/controls/dev/dll-tabular/XamlMetadataProviderGenerated.tt @@ -205,6 +205,11 @@ struct Entry { WriteLine("#include \"" + type.Name + ".properties.h\""); } + + // TableView's cell-tooltip ownership property is registered lazily outside the generated + // TableViewProperties, so the generated file owns its include as well as the clear call below -- + // otherwise the generated header would only compile when its includer happened to add it first. + WriteLine("#include \"TableViewToolTipHelpers.h\""); // The Tabular controls DLL hosts only TableView + its primitives; the MUXC-only helpers // (AutoSuggestBoxHelper / ComboBoxHelper / RecyclePool / RevealBrush) that the full MUXC @@ -224,6 +229,11 @@ namespace { // The Tabular controls DLL hosts only TableView + its primitives; the MUXC-only helpers // (AutoSuggestBoxHelper / ComboBoxHelper / RecyclePool / RevealBrush) are intentionally omitted. + // + // TableView's cell-tooltip ownership property is registered lazily outside the generated + // TableViewProperties, so it needs clearing here too or a XAML re-init would keep handing + // out a registration from the dead core. + WriteLine("TableViewDetails::ClearCellToolTipProperties();"); }); WriteLine(""); #> diff --git a/docs/api-specs/TableView/TableView-spec.md b/docs/api-specs/TableView/TableView-spec.md index bc797aae4d..3ead33a146 100644 --- a/docs/api-specs/TableView/TableView-spec.md +++ b/docs/api-specs/TableView/TableView-spec.md @@ -445,6 +445,7 @@ Named to match `ItemsView`, which ships `Select` / `Deselect` / `IsSelected` ove | Event | Args | Description | |---|---|---| +| `CellToolTipRequested` | `TableViewCellToolTipRequestedEventArgs` | Raised when a cell's tooltip is resolved — on realization, on cell rebuild, and on `InvalidateCellToolTips()`. May be raised repeatedly for the same cell and item, so handlers should be cheap and side-effect free. Opt-in: with no handler attached the control does no per-cell work. `InvalidateCellToolTips()` queues a coalesced re-resolve for every realized cell, for handlers attached after realization or when the underlying data changes. | | `SelectionChanged` | `SelectionChangedEventArgs` | Raised after the selection has settled. Reading `SelectedItem`, `SelectedIndex`, or a row's `IsSelected` inside the handler observes the new state. Replacing a selection raises **one** event carrying both the removed and the added item. | Selection is raised only for real changes: a re-select of the already-selected row, or an index shift caused by a collection reshape, does not raise `SelectionChanged`. @@ -647,6 +648,7 @@ How an edit is being closed. |---|---| | `TableViewBeginningEditEventArgs` | `Item`, `Column` (read-only); `Cancel` (settable) | | `TableViewCellEditEndingEventArgs` | `Item`, `Column`, `EditAction` (read-only); `Cancel` (settable) | +| `TableViewCellToolTipRequestedEventArgs` | `Item`, `Column` (read-only); `Content`, `ToolTipHelpText` (settable) | ## Selection event args @@ -730,6 +732,15 @@ namespace Microsoft.UI.Xaml.Controls.Tabular Boolean Cancel; }; + [MUX_PREVIEW, webhosthidden] + runtimeclass TableViewCellToolTipRequestedEventArgs + { + Object Item { get; }; + Microsoft.UI.Xaml.Controls.Tabular.TableViewColumn Column { get; }; + Object Content { get; set; }; + String ToolTipHelpText { get; set; }; + }; + [MUX_PREVIEW, webhosthidden] runtimeclass TableViewCellEditEndingEventArgs { @@ -837,6 +848,8 @@ namespace Microsoft.UI.Xaml.Controls.Tabular Boolean IsSelected(Int32 index); void DeselectAll(); + event Windows.Foundation.TypedEventHandler CellToolTipRequested; + void InvalidateCellToolTips(); event Windows.Foundation.TypedEventHandler SelectionChanged; static Microsoft.UI.Xaml.DependencyProperty ItemsSourceProperty { get; }; diff --git a/docs/design-notes/TabularControls/TableView-functional-spec.md b/docs/design-notes/TabularControls/TableView-functional-spec.md index 9390d46539..6460f1fefa 100644 --- a/docs/design-notes/TabularControls/TableView-functional-spec.md +++ b/docs/design-notes/TabularControls/TableView-functional-spec.md @@ -75,6 +75,15 @@ Each item notes the MLP/v1 vs deferred status and the delivering PR. - Light / Dark / High Contrast theme tokens + inline fallbacks (`#29FFFFFF` dark gridline). — **PR2** (full self-themed Theme-XBF emission deferred) - Cell-level styling: custom cells via `TableViewTemplateColumn` + built-in text-cell defaults (left, vertically centered). A public per-column alignment/weight API — **deferred (PR3+)**. +### Tooltips +- Cell: opt-in via `CellToolTipRequested`; the app supplies content per cell on demand. No handler means no tooltip and no per-cell cost. Needed because text cells render with `CharacterEllipsis` and no wrapping, so over-wide values are otherwise unreadable. +- Author precedence: a tooltip set inside a cell's own content template opens over that content; the control's tooltip covers the rest of the cell, and the control never touches a tooltip it did not attach. +- Accessibility: the tooltip text is published as the cell's `AutomationProperties.HelpText` unless the cell already reports it, so the information is not pointer-only and is not announced twice. For content UIA cannot read the handler sets `ToolTipHelpText`, since the cell wrapper is not reachable from the app. +- Recycling: a recycled row never shows a previous item's cell tooltip, including after the last handler is removed. `InvalidateCellToolTips()` queues a coalesced re-resolve of every realized cell when a handler is attached late or the underlying data changes. +- Column-header and group-header tooltips are **deferred**: the header band is `IsHitTestVisible="False"` (`TableView.xaml`) so a header tooltip could never open, and grouping is not wired up on `main`. Both land with the work that makes those surfaces interactive. + +> Tooltips are **not** gated on text truncation. No WinUI control keys tooltips off `IsTextTrimmed`; the shipped pattern is to gate on a cheap content predicate (non-empty string) or an explicit opt-in. + ### Accessibility (UIA) - Grid/Table peers; Row peer (`SelectionItem` + `GridItem`); ColumnHeader peer (`Invoke` → sort). — **PR2** read-only Grid/Table · **PR3** Selection + sort invoke - Narrator, keyboard navigation parity, sort announcements. — **PR2** nav · **PR3** sort From 281baa9600431ec6ce0d634cf08e7d05e7229f72 Mon Sep 17 00:00:00 2001 From: Hitesh Kumar Date: Thu, 27 Aug 2026 17:23:01 +0530 Subject: [PATCH 2/5] Tabular: address review findings on cell tooltips Contain handler exceptions per row on the deferred invalidate path, which previously let a throwing handler escape into a dispatcher callback. Drain a handler-queued rebuild from refreshes reached directly (invalidate, edit close); only the rebuild path drained, leaving those rows stamped for the previous state. Test ownership against the raw attached value: ToolTipService stores whatever the app set, and a bare string was read as an empty slot and overwritten. Reject a ToolTip as Content rather than nesting it, and correct the IDL, which claimed it overrides placement. Keep the invalidate queued flag set across the pass and record a follow-up, so a handler calling it cannot queue a callback every tick. Release tooltip content from OnRowElementClearing: the release path was unreachable because recycle-out never rebuilds, so app content stayed alive in the pool. Retract the tooltip when an edit begins and re-resolve when one is abandoned, covering forced closes that do not rebuild. Keep the owned-tooltip flag true unless the pass completed, so a handler throwing on the first cell cannot hide earlier tooltips from cleanup. Resolve the cell's UIA text only when there is something to publish; it can allocate an automation peer. --- controls/dev/TableView/TableView.cpp | 41 ++++++--- controls/dev/TableView/TableView.h | 2 + controls/dev/TableView/TableView.idl | 33 ++++--- controls/dev/TableView/TableViewRow.cpp | 76 +++++++++------- controls/dev/TableView/TableViewRow.h | 5 +- .../dev/TableView/TableViewToolTipHelpers.h | 89 +++++++------------ .../dev/dll-tabular/XamlMetadataProvider.cpp | 2 +- 7 files changed, 129 insertions(+), 119 deletions(-) diff --git a/controls/dev/TableView/TableView.cpp b/controls/dev/TableView/TableView.cpp index b198d1a4da..31994f1456 100644 --- a/controls/dev/TableView/TableView.cpp +++ b/controls/dev/TableView/TableView.cpp @@ -1135,9 +1135,7 @@ winrt::com_ptr TableView::RaiseCellToolT const winrt::TableViewColumn& column, const winrt::IInspectable& item) { - // Handler exceptions are deliberately NOT swallowed: every other event on this control lets - // them propagate so the XAML core surfaces them, and a silently missing tooltip on some rows is - // far harder to diagnose than the crash. + // Contained by the callers, which run from framework callbacks. auto args = winrt::make_self(item, column); m_cellToolTipRequestedEventSource(*this, *args); return args; @@ -1145,12 +1143,12 @@ winrt::com_ptr TableView::RaiseCellToolT void TableView::InvalidateCellToolTips() { - // Deferred rather than raising here: this is public, so an app can call it from inside its own - // CellToolTipRequested handler, and raising synchronously would re-enter the pass and walk the - // repeater's live children while app code is mutating them. Coalesces a burst into one pass, - // matching LinedFlowLayout::InvalidateItemsInfo. + // Deferred: this is public, so an app can call it from its own handler. Raising synchronously + // would re-enter the pass and walk live children mid-mutation. Matches InvalidateItemsInfo. if (m_cellToolTipRefreshQueued) { + // Already queued or running: coalesce into exactly one follow-up. + m_cellToolTipRefreshDirty = true; return; } @@ -1167,8 +1165,16 @@ void TableView::InvalidateCellToolTips() { if (auto strongThis = weakThis.get()) { - strongThis->m_cellToolTipRefreshQueued = false; + // The flag stays set across the pass, or a handler calling back would loop forever. + strongThis->m_cellToolTipRefreshDirty = false; strongThis->RefreshCellToolTipsOnRealizedRows(); + strongThis->m_cellToolTipRefreshQueued = false; + + if (strongThis->m_cellToolTipRefreshDirty) + { + strongThis->m_cellToolTipRefreshDirty = false; + strongThis->InvalidateCellToolTips(); + } } })) { @@ -1181,7 +1187,17 @@ void TableView::RefreshCellToolTipsOnRealizedRows() { ForEachRealizedRow([](winrt::TableViewRow const& row) { - winrt::get_self(row)->RefreshCellToolTips(); + try + { + winrt::get_self(row)->RefreshCellToolTips(); + } + catch (...) + { + // Contained per row: an exception escaping a dispatcher callback fail-fasts, and one + // bad row must not abort the rest. + TVDiag::LogRetailF(L"[TableView] CellToolTipRequested handler threw (HRESULT 0x%08X). Continuing.", + static_cast(winrt::to_hresult())); + } }); } void TableView::OnRowElementClearing( @@ -1216,7 +1232,10 @@ void TableView::OnRowElementClearing( } } - winrt::get_self(row)->SetOwningTableViewInternal(nullptr); + auto const rowImpl = winrt::get_self(row); + // Release app-supplied tooltip content rather than pinning it in the recycle pool. + rowImpl->ReleaseCellToolTips(); + rowImpl->SetOwningTableViewInternal(nullptr); InvalidateMeasure(); } } @@ -1429,4 +1448,4 @@ void TableView::OnTableViewUnloaded() // while detached. m_themeSettingsChangedRevoker.revoke(); m_themeSettings = nullptr; -} \ No newline at end of file +} diff --git a/controls/dev/TableView/TableView.h b/controls/dev/TableView/TableView.h index 35b1a83782..5023cd4451 100644 --- a/controls/dev/TableView/TableView.h +++ b/controls/dev/TableView/TableView.h @@ -573,6 +573,8 @@ class TableView : bool m_rebuildHeadersQueued{ false }; // A coalesced cell-tooltip re-resolve is pending on the dispatcher (InvalidateCellToolTips). bool m_cellToolTipRefreshQueued{ false }; + // An invalidate arrived while a pass was queued or running; coalesced into one follow-up. + bool m_cellToolTipRefreshDirty{ false }; // Per-instance resource cache; replaces the former process-global map keyed by `this`. TableViewResourceCache m_resourceCache{}; diff --git a/controls/dev/TableView/TableView.idl b/controls/dev/TableView/TableView.idl index dfec290e13..4d4f5c15ca 100644 --- a/controls/dev/TableView/TableView.idl +++ b/controls/dev/TableView/TableView.idl @@ -185,8 +185,7 @@ runtimeclass TableViewCellEditEndingEventArgs Boolean Cancel; }; -// Raised while a cell is being realized so the app can supply that cell's tooltip. -// Opt-in: with no handler attached the control does no per-cell work. +// Args for CellToolTipRequested. [MUX_PREVIEW] [webhosthidden] runtimeclass TableViewCellToolTipRequestedEventArgs @@ -194,17 +193,16 @@ runtimeclass TableViewCellToolTipRequestedEventArgs // The data item of the row the cell belongs to. Object Item { get; }; - // The column of the cell being realized. Always non-null. + // The column of the cell. Always non-null. MU_XC_NAMESPACE.TableViewColumn Column { get; }; - // Set by the handler to the tooltip content. Any non-null value is honoured, including a - // configured ToolTip (the way to override placement) or non-string content such as a panel; - // leaving it null (the default) means "no tooltip for this cell". + // The tooltip content: a string, or any content a ToolTip can host. Null or an empty string + // means no tooltip. A ToolTip is not valid content -- the control owns the ToolTip and its + // placement. Object Content { get; set; }; - // Accessible text for this cell's tooltip, surfaced as the cell's UIA HelpText. Set this when - // Content is not a string -- non-string content cannot be read by assistive technology, and - // the cell wrapper is not reachable from the app. String Content is used automatically. + // Accessible text, published as the cell's UIA HelpText. Set it when Content is not a string; + // it overrides string Content, and is ignored when Content is null. String ToolTipHelpText { get; set; }; }; @@ -469,17 +467,16 @@ unsealed runtimeclass TableView : Microsoft.UI.Xaml.Controls.Control event Windows.Foundation.TypedEventHandler SelectionChanged; // ----- Tooltips ----- - // Raised when a cell's tooltip is resolved -- on realization, on cell rebuild, and on - // InvalidateCellToolTips. May be raised repeatedly for the same cell and item, so handlers - // should be cheap and free of side effects. Handlers set args.Content; any non-null content - // produces the tooltip. A tooltip set inside a cell's own content template opens over that - // content, and the control's cell tooltip covers the rest of the cell. Not raised when no - // handler is attached; handlers added or removed after rows are realized take effect as those - // rows are re-realized, or on the next InvalidateCellToolTips. + // Raised as each cell's tooltip is resolved: on realization, on cell rebuild, and on + // InvalidateCellToolTips. May be raised repeatedly for the same cell, so handlers should be + // cheap and side-effect free. Opt-in -- not raised when no handler is attached. A tooltip the + // app sets inside a cell's own content template opens over that content; the control's tooltip + // covers the rest of the cell. event Windows.Foundation.TypedEventHandler CellToolTipRequested; - // Re-raises CellToolTipRequested for every realized cell. Call after attaching a handler to a - // TableView whose rows are already realized, or when the data behind the tooltips changes. + // Queues a coalesced re-resolve of every realized cell; it does not run before this call + // returns. Use it after attaching a handler to a TableView whose rows are already realized, or + // when the data behind the tooltips changes. Safe to call from a handler. void InvalidateCellToolTips(); static Microsoft.UI.Xaml.DependencyProperty ItemsSourceProperty { get; }; diff --git a/controls/dev/TableView/TableViewRow.cpp b/controls/dev/TableView/TableViewRow.cpp index 1140a4c461..ec9aa262e4 100644 --- a/controls/dev/TableView/TableViewRow.cpp +++ b/controls/dev/TableView/TableViewRow.cpp @@ -545,9 +545,8 @@ void TableViewRow::InvalidateCells() void TableViewRow::RebuildCells() { - // The tooltip pass reaches app code, so it must not run under the re-entry guard: a handler - // that replaces Columns re-enters here, and a swallowed rebuild would leave the row empty with - // nothing queued to retry it. + // The tooltip pass calls app code, so it runs outside the re-entry guard. A handler can drop the + // last reference to this row. auto strongThis = get_strong(); if (m_isRebuildingCells || m_isRefreshingCellToolTips) @@ -556,18 +555,16 @@ void TableViewRow::RebuildCells() return; } - // One rebuild, plus replays for app code that mutates the row from a tooltip handler; the - // tooltip pass itself can queue one, so it runs inside the drain. Beyond the cap the app is - // livelocking the UI thread, so the request is dropped with a diagnostic rather than spun on. + // One rebuild plus replays for app code that mutates the row from a handler. Beyond the cap the + // app is livelocking the UI thread, so the request is dropped with a diagnostic. constexpr int c_maxRebuildDrainPasses = 3; for (int pass = 0; pass < c_maxRebuildDrainPasses; ++pass) { m_rebuildCellsPending = false; RebuildCellsCore(); - // Contained here rather than at the raise: this runs from framework callbacks - // (OnApplyTemplate, OnDataContextChanged, the repeater's measure pass), where letting an app - // handler's exception cross back into XAML is a fail-fast rather than a recoverable error. + // Contained here: this runs from framework callbacks, where an escaping handler exception is + // a fail-fast rather than a recoverable error. try { RefreshCellToolTips(); @@ -849,9 +846,19 @@ void TableViewRow::RebuildCellsCore() void TableViewRow::RefreshCellToolTips() { - // The guard lives here, not at the caller: this is the function that calls into app code, and it - // has more than one entry point (cell rebuild, and TableView::InvalidateCellToolTips). A handler - // that triggers either one re-enters, and without this it would recurse until the stack died. + RefreshCellToolTipsCore(); + + // Only RebuildCells drains, so a refresh reached directly (invalidate, edit close) must drain the + // rebuild a handler queued, or the row is left stamped for the previous state. + if (m_rebuildCellsPending && !m_isRebuildingCells && !m_isRefreshingCellToolTips) + { + RebuildCells(); + } +} + +void TableViewRow::RefreshCellToolTipsCore() +{ + // Guarded here, not at the caller: this is what calls app code, and it has several entry points. if (m_isRefreshingCellToolTips) { return; @@ -868,7 +875,7 @@ void TableViewRow::RefreshCellToolTips() auto refreshGuard = wil::scope_exit([this]() { m_isRefreshingCellToolTips = false; }); auto const owner = GetOwningTableView(); - // Recycled out: the row keeps its cells for the next prepare, but an owned tooltip can hold + // No owner: the row keeps its cells for the next prepare, but an owned tooltip can hold // app-supplied content, so release it rather than pinning it in the pool. if (!owner) { @@ -878,9 +885,7 @@ void TableViewRow::RefreshCellToolTips() auto const ownerImpl = winrt::get_self(owner); - // Opt-in: with no handler this costs one event_source test per row and allocates nothing -- - // unless a previous pass left tooltips behind, in which case they must be retracted or a - // recycled cell would keep showing the previous item's text. + // Opt-in: with no handler this is one event_source test, unless a previous pass left tooltips. if (!ownerImpl->HasCellToolTipHandler()) { if (m_hasOwnedCellToolTips) @@ -890,9 +895,8 @@ void TableViewRow::RefreshCellToolTips() return; } - // The raise reaches app code, which may mutate Columns or the cell children, so the targets are - // snapshotted first rather than walking the live collection. The scratch vector is moved into a - // local so a nested pass cannot clear it while this one iterates; the capacity is handed back. + // Snapshot before raising: a handler may mutate Columns or the cell children. The scratch vector + // is moved to a local so a nested pass cannot clear it mid-iteration; capacity is handed back. auto targets = std::move(m_toolTipTargets); targets.clear(); auto const children = host.Children(); @@ -901,16 +905,14 @@ void TableViewRow::RefreshCellToolTips() auto const editingWrapper = m_editingCellWrapper.get(); for (uint32_t i = 0; i < count; ++i) { - // Target the wrapper, not the display element: the wrapper fills the cell, so the whole - // cell is the hover target, and it is also the UIA cell node. + // The wrapper, not the display element: it fills the cell and is the UIA cell node. auto const cellWrapper = children.GetAt(i).try_as(); if (!cellWrapper) { continue; } - // An open editor owns its cell: a tooltip over a live text box is noise. Retract rather - // than skip -- skipping would leave an enabled tooltip behind that no later pass clears. + // An open editor owns its cell; retract rather than skip, or the tooltip is left enabled. if (cellWrapper == editingWrapper) { TableViewDetails::ClearOwnedToolTip(cellWrapper); @@ -923,12 +925,13 @@ void TableViewRow::RefreshCellToolTips() } } - // Commit the ownership flag and hand the buffer back on every exit, including the exception path - // -- a handler is allowed to throw, and a row that has tooltips must know to clear them later. + // Commit on every exit, including the throw path. A handler can throw before any cell is visited, + // so the flag stays conservatively true unless the pass completed. bool anyOwned = false; + bool passCompleted = false; auto passGuard = wil::scope_exit([&]() { - m_hasOwnedCellToolTips = anyOwned; + m_hasOwnedCellToolTips = passCompleted ? anyOwned : true; targets.clear(); m_toolTipTargets = std::move(targets); }); @@ -941,15 +944,11 @@ void TableViewRow::RefreshCellToolTips() try { - // Contained separately from the raise -- a handler's own exception propagates, but - // app-supplied content may be an already-parented element, which throws on assignment. - // The UIA text is only needed to suppress a duplicate announcement, so it is resolved - // lazily rather than for every cell including the ones with no tooltip. + // Separate from the raise: app content may be already parented, which throws on assign. const bool owned = content ? TableViewDetails::SetOwnedToolTip( target.second, content, - GetCellValueTextFromWrapper(target.second), args->ToolTipHelpText(), winrt::PlacementMode::Mouse) : (TableViewDetails::ClearOwnedToolTip(target.second), false); @@ -964,6 +963,16 @@ void TableViewRow::RefreshCellToolTips() static_cast(winrt::to_hresult())); } } + + passCompleted = true; +} + +void TableViewRow::ReleaseCellToolTips() +{ + if (auto const host = m_cellsHost.get(); host && m_hasOwnedCellToolTips) + { + ClearOwnedCellToolTips(host); + } } void TableViewRow::ClearOwnedCellToolTips(const winrt::Panel& host) @@ -1169,6 +1178,8 @@ bool TableViewRow::BeginCellEdit(const winrt::TableViewColumn& column, const win m_editingColumn.set(column); m_editingCellWrapper.set(cellWrapper); + // An editor owns its cell; a tooltip over a live text box is noise. + TableViewDetails::ClearOwnedToolTip(cellWrapper); m_editingElement.set(editingElement); // The column decides how its editor is primed - focus, caret, selection are editor-specific, @@ -1282,6 +1293,9 @@ void TableViewRow::AbandonCellEdit() m_editingCellWrapper.set(nullptr); m_editingElement.set(nullptr); m_editingDisplayElement.set(nullptr); + + // The cell is a display cell again; re-resolve the tooltip the edit retracted. + RefreshCellToolTips(); } // Pointer entry point for editing, and the only place a pointer establishes the current cell. diff --git a/controls/dev/TableView/TableViewRow.h b/controls/dev/TableView/TableViewRow.h index 42ab23267c..2a3f9d53dd 100644 --- a/controls/dev/TableView/TableViewRow.h +++ b/controls/dev/TableView/TableViewRow.h @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. See LICENSE in the project root for license information. #pragma once @@ -62,6 +62,8 @@ class TableViewRow : // Internal: re-resolves this row's cell tooltips against its current data item. Raised outside // the cell-rebuild re-entry guard because it calls into app code. void RefreshCellToolTips(); + // Releases control-owned cell tooltips (recycle-out). + void ReleaseCellToolTips(); // Apply a column's current visibility to this row's matching cell (the row owns its cells, // so TableView asks the row instead of reaching into the row's cell panel). The visibility is @@ -138,6 +140,7 @@ class TableViewRow : void RebuildCells(); // The guarded body. RebuildCells wraps it so the tooltip pass runs with the guard released. void RebuildCellsCore(); + void RefreshCellToolTipsCore(); void ClearOwnedCellToolTips(const winrt::Panel& host); // Coalesces a burst of Columns-collection changes into a single cell rebuild on the next diff --git a/controls/dev/TableView/TableViewToolTipHelpers.h b/controls/dev/TableView/TableViewToolTipHelpers.h index 5cf29f59b7..e2a78d384d 100644 --- a/controls/dev/TableView/TableViewToolTipHelpers.h +++ b/controls/dev/TableView/TableViewToolTipHelpers.h @@ -3,33 +3,23 @@ #pragma once -#include "pch.h" -#include "common.h" #include "GlobalDependencyProperty.h" -#include "CppWinRTHelpers.h" +#include "TableViewAutomationHelpers.h" #include -// Tooltip plumbing for the cell realization path. Kept out of TableView so the helpers stay element -// operations with no control state, next to TableViewAutomationHelpers.h / TableViewGroupingHelpers.h. namespace TableViewDetails { - // Everything the control needs to know about the tooltip it put on an element: the object it - // attached, and the exact HelpText it published for it. Both facts have to be recorded rather - // than re-derived -- the published text is not always the tooltip's content (an app can supply - // its own accessible text for non-string content), so deriving it would strand stale HelpText - // on a recycled cell. + // The tooltip the control attached to an element, and the exact HelpText it published for it. + // Both are recorded: the published text is not always the tooltip's content, so re-deriving it + // would strand stale HelpText on a recycled cell. struct CellToolTipRecord : winrt::implements { winrt::ToolTip ToolTip{ nullptr }; winrt::hstring PublishedHelpText{}; }; - // Deliberately not in the IDL, so an app cannot reach or overwrite the record -- unlike the - // ToolTip's public Tag, which an app can reach through ToolTipService::GetToolTip. - // - // GlobalDependencyProperty rather than a winrt::DependencyProperty static: it must not run a - // destructor at DLL unload, and it has to be re-registerable after ClearProperties. + // Not in the IDL: an app can reach the ToolTip's Tag, but not this. inline GlobalDependencyProperty s_cellToolTipRecordProperty{ nullptr }; inline winrt::DependencyProperty EnsureCellToolTipRecordProperty() @@ -48,9 +38,7 @@ namespace TableViewDetails return s_cellToolTipRecordProperty; } - // Paired with EnsureCellToolTipRecordProperty, and called from the same teardown as the - // generated TableViewProperties::ClearProperties so a XAML re-init re-registers against the new - // core rather than handing out a registration from the dead one. + // Called from the generated ClearTypeProperties, so a XAML re-init re-registers against the new core. inline void ClearCellToolTipProperties() { s_cellToolTipRecordProperty = nullptr; @@ -85,7 +73,6 @@ namespace TableViewDetails return std::nullopt; } - // Retracts the HelpText this control published for the element, if it is still the value there. inline void RetractPublishedHelpText(const winrt::FrameworkElement& element, CellToolTipRecord& record) { if (!record.PublishedHelpText.empty() && @@ -97,9 +84,8 @@ namespace TableViewDetails record.PublishedHelpText = {}; } - // Retracts the tooltip this control put on the element. The ToolTip object stays attached and is - // neutralized rather than detached, so a recycled cell reuses it instead of allocating a fresh - // Control per cell per scroll -- matching TabViewItem and NavigationViewItem. + // The ToolTip stays attached and is neutralized rather than detached, so a recycled cell reuses + // it instead of allocating a Control per cell per scroll. Matches TabViewItem. inline void ClearOwnedToolTip(const winrt::FrameworkElement& element) { auto const record = GetRecord(element); @@ -110,13 +96,10 @@ namespace TableViewDetails RetractPublishedHelpText(element, *record); - // Still ours only if the object we recorded is the one currently attached; otherwise the app - // replaced or removed it and we must not touch what is there now. if (record->ToolTip && record->ToolTip == winrt::ToolTipService::GetToolTip(element).try_as()) { - // Disable before dropping the content: clearing the content of a tooltip that is - // currently open removes a live popup's child, which is the shape behind the reentrant - // CPopup::RemoveChild teardown crash. + // Disable and close before dropping content: clearing an open tooltip's content removes a + // live popup's child, the shape behind the reentrant CPopup::RemoveChild crash. record->ToolTip.IsEnabled(false); if (record->ToolTip.IsOpen()) { @@ -126,20 +109,16 @@ namespace TableViewDetails } else { + // The app replaced or removed it; leave what is there now alone. ForgetRecord(element); } } - // Applies app-supplied tooltip content to a control-created cell wrapper, and returns whether the - // element is left carrying a control-owned tooltip. Never touches a tooltip the app set itself. - // - // `existingUiaText` is what the element already reports to UIA; `helpTextOverride` is the app's - // accessible text for content UIA cannot read. The resulting text is published as HelpText unless - // the element already reports it, so assistive technology gets the information exactly once. + // Applies app-supplied content to a control-created cell wrapper. Returns whether the element is + // left carrying a control-owned tooltip. Never touches a tooltip the app set itself. inline bool SetOwnedToolTip( const winrt::FrameworkElement& element, const winrt::IInspectable& content, - const winrt::hstring& existingUiaText, const winrt::hstring& helpTextOverride, winrt::PlacementMode placement) { @@ -149,12 +128,13 @@ namespace TableViewDetails } auto record = GetRecord(element); - auto const existing = winrt::ToolTipService::GetToolTip(element).try_as(); - auto const owned = (record && record->ToolTip && record->ToolTip == existing) ? record->ToolTip : nullptr; + // The raw value, not a ToolTip-narrowed one: ToolTipService stores whatever the app set, and + // the common form is a bare string. Narrowing first would read that as an empty slot. + auto const existingValue = winrt::ToolTipService::GetToolTip(element); + auto const owned = (record && record->ToolTip && record->ToolTip == existingValue.try_as()) + ? record->ToolTip : nullptr; - // The app set or replaced the tooltip on this element: retract what we published and stop - // treating the element as ours. - if (existing && !owned) + if (existingValue && !owned) { if (record) { @@ -164,9 +144,9 @@ namespace TableViewDetails return false; } + // A ToolTip as content would render nested inside ours, and the control owns placement. auto const text = TryGetString(content); - // An empty string is never content -- it would pop an empty tooltip. - if (!content || (text && text->empty())) + if (!content || (text && text->empty()) || content.try_as()) { ClearOwnedToolTip(element); return false; @@ -179,14 +159,11 @@ namespace TableViewDetails element.SetValue(s_cellToolTipRecordProperty, *record); } - // Retract the previous pairing before the content changes, or HelpText published for the - // previous item would outlive it. RetractPublishedHelpText(element, *record); if (owned) { - // Neutralize first so a throwing assignment cannot leave the previous item's content - // enabled on a recycled cell, and so the content is never swapped under an open popup. + // Neutralize first: a throwing assignment must not leave the previous item's content live. owned.IsEnabled(false); if (owned.IsOpen()) { @@ -203,24 +180,22 @@ namespace TableViewDetails toolTip.Content(content); toolTip.Placement(placement); - // Record before attaching. The other order leaves an attached tooltip with no record if - // the write throws, which the next pass would read as app-set and never clear; this way a - // throw leaves a record for a tooltip that is not attached, which reads as "not ours". + // Record before attaching: a throw then reads as "not ours" rather than orphaning a + // tooltip nothing can clear. record->ToolTip = toolTip; winrt::ToolTipService::SetToolTip(element, toolTip); } - // Publish the accessible text unless the element already reports it, so Narrator does not read - // the same string twice. Only text this control published is ever overwritten. + // Publish unless the cell already reports the text, so Narrator does not read it twice. The + // cell's UIA text is resolved here, not by the caller: it can allocate a peer, and cells with + // no tooltip never need it. auto const helpText = !helpTextOverride.empty() ? helpTextOverride : (text ? *text : winrt::hstring{}); - if (!helpText.empty() && helpText != existingUiaText) + if (!helpText.empty() && + helpText != GetCellValueTextFromWrapper(element) && + winrt::AutomationProperties::GetHelpText(element).empty()) { - auto const current = winrt::AutomationProperties::GetHelpText(element); - if (current.empty()) - { - winrt::AutomationProperties::SetHelpText(element, helpText); - record->PublishedHelpText = helpText; - } + winrt::AutomationProperties::SetHelpText(element, helpText); + record->PublishedHelpText = helpText; } return true; diff --git a/controls/dev/dll-tabular/XamlMetadataProvider.cpp b/controls/dev/dll-tabular/XamlMetadataProvider.cpp index c6f630d16f..51322afe0a 100644 --- a/controls/dev/dll-tabular/XamlMetadataProvider.cpp +++ b/controls/dev/dll-tabular/XamlMetadataProvider.cpp @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. See LICENSE in the project root for license information. #include "pch.h" From e5e01e5ceabea5f9237102af3e4c01396723f863 Mon Sep 17 00:00:00 2001 From: Hitesh Kumar Date: Fri, 28 Aug 2026 01:54:30 +0530 Subject: [PATCH 3/5] Tabular: bound the cell tooltip drains and contain the raise The rebuild drain called the public RefreshCellToolTips, which drains as well, so a handler that dirtied the row started a nested drain with a fresh counter instead of tripping the cap. Depth was bounded only by how long the app kept mutating, and the "dropping a pending cell rebuild" diagnostic was unreachable on the path it was written for. Both loops now run over the Core primitives and share one pass cap, so a handler gets the same number of replays whichever entry point it reaches. Bound the deferred invalidate the same way. A handler calling InvalidateCellToolTips on every raise re-armed the follow-up forever, queuing a dispatcher callback per tick and consuming a core; beyond the cap the request is now dropped with a diagnostic. Hold the coalescing flag across the synchronous fallbacks, which ran without it, so a handler reaching them re-entered and recursed rather than recording a follow-up. Clear the flag on the throw path too: it was a bare assignment, and a pass that threw left every later invalidate coalescing into a callback that would never run. Raise CellToolTipRequested inside the try that applies its result. Only the apply was contained, so a throwing handler escaped through the edit-close paths, which run from key and focus callbacks. Test the cheap conditions before resolving the cell's UIA text, which can allocate an automation peer for non-text content, and correct the IDL: calling the invalidate from a handler is re-entrancy-safe, but it is not free to call unconditionally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- controls/dev/TableView/TableView.cpp | 49 +++++++++- controls/dev/TableView/TableView.h | 4 + controls/dev/TableView/TableView.idl | 4 +- controls/dev/TableView/TableViewRow.cpp | 97 +++++++++++++------ controls/dev/TableView/TableViewRow.h | 2 + .../dev/TableView/TableViewToolTipHelpers.h | 8 +- 6 files changed, 125 insertions(+), 39 deletions(-) diff --git a/controls/dev/TableView/TableView.cpp b/controls/dev/TableView/TableView.cpp index 31994f1456..ded08630a9 100644 --- a/controls/dev/TableView/TableView.cpp +++ b/controls/dev/TableView/TableView.cpp @@ -24,6 +24,10 @@ static constexpr std::wstring_view s_HeaderGridLineName{ L"TableViewHeaderGridLi namespace { + // Consecutive follow-up tooltip passes a handler may request by calling InvalidateCellToolTips + // from inside CellToolTipRequested. Matches the row's rebuild drain cap. + constexpr int c_maxCellToolTipRefreshPasses = 3; + winrt::ScrollViewer FindScrollViewerAncestor(winrt::DependencyObject const& start) { winrt::DependencyObject node = start; @@ -1155,7 +1159,9 @@ void TableView::InvalidateCellToolTips() auto dispatcher = DispatcherQueue(); if (!dispatcher) { - RefreshCellToolTipsOnRealizedRows(); + // No dispatcher to defer onto. The flag is still held across the synchronous pass so a + // handler calling back coalesces into a follow-up instead of re-entering and recursing. + RefreshCellToolTipsSynchronously(); return; } @@ -1165,24 +1171,57 @@ void TableView::InvalidateCellToolTips() { if (auto strongThis = weakThis.get()) { - // The flag stays set across the pass, or a handler calling back would loop forever. + // The flag stays set across the pass, or a handler calling back would queue a + // callback every tick. Cleared on the throw path too: leaving it set would silently + // disable every future invalidate. strongThis->m_cellToolTipRefreshDirty = false; - strongThis->RefreshCellToolTipsOnRealizedRows(); + try + { + strongThis->RefreshCellToolTipsOnRealizedRows(); + } + catch (...) + { + TVDiag::LogRetailF(L"[TableView] Cell-tooltip re-resolve failed (HRESULT 0x%08X).", + static_cast(winrt::to_hresult())); + } strongThis->m_cellToolTipRefreshQueued = false; if (strongThis->m_cellToolTipRefreshDirty) { strongThis->m_cellToolTipRefreshDirty = false; - strongThis->InvalidateCellToolTips(); + + // A handler that invalidates on every raise would otherwise re-arm this forever, + // pegging the UI thread with a dispatcher callback per tick. Bounded like the + // row's rebuild drain: beyond the cap the request is dropped with a diagnostic. + if (++strongThis->m_cellToolTipRefreshPasses < c_maxCellToolTipRefreshPasses) + { + strongThis->InvalidateCellToolTips(); + return; + } + + TVDiag::LogRetailF(L"[TableView] Dropping a cell-tooltip re-resolve after %d passes; " + L"a CellToolTipRequested handler is calling InvalidateCellToolTips unconditionally.", + c_maxCellToolTipRefreshPasses); } + + strongThis->m_cellToolTipRefreshPasses = 0; } })) { m_cellToolTipRefreshQueued = false; - RefreshCellToolTipsOnRealizedRows(); + RefreshCellToolTipsSynchronously(); } } +// Runs a pass with the coalescing flag held, so a handler that calls InvalidateCellToolTips records +// a follow-up instead of re-entering this synchronously and recursing without bound. +void TableView::RefreshCellToolTipsSynchronously() +{ + m_cellToolTipRefreshQueued = true; + auto queuedGuard = wil::scope_exit([this]() { m_cellToolTipRefreshQueued = false; }); + + RefreshCellToolTipsOnRealizedRows(); +} void TableView::RefreshCellToolTipsOnRealizedRows() { ForEachRealizedRow([](winrt::TableViewRow const& row) diff --git a/controls/dev/TableView/TableView.h b/controls/dev/TableView/TableView.h index 5023cd4451..5c13377088 100644 --- a/controls/dev/TableView/TableView.h +++ b/controls/dev/TableView/TableView.h @@ -153,6 +153,7 @@ class TableView : // Queues a coalesced tooltip re-resolve for every realized cell (public, from the IDL). void InvalidateCellToolTips(); void RefreshCellToolTipsOnRealizedRows(); + void RefreshCellToolTipsSynchronously(); // Requested by a cell panel (header/row) during measure when a realized cell's own measured width // changed (grow or shrink). Invalidates our measure synchronously so ResolveColumnWidths re-runs in @@ -575,6 +576,9 @@ class TableView : bool m_cellToolTipRefreshQueued{ false }; // An invalidate arrived while a pass was queued or running; coalesced into one follow-up. bool m_cellToolTipRefreshDirty{ false }; + // Consecutive handler-requested follow-up passes, so an unconditionally-invalidating handler is + // dropped with a diagnostic instead of pegging the UI thread forever. + int m_cellToolTipRefreshPasses{ 0 }; // Per-instance resource cache; replaces the former process-global map keyed by `this`. TableViewResourceCache m_resourceCache{}; diff --git a/controls/dev/TableView/TableView.idl b/controls/dev/TableView/TableView.idl index 4d4f5c15ca..c40ba6cd74 100644 --- a/controls/dev/TableView/TableView.idl +++ b/controls/dev/TableView/TableView.idl @@ -476,7 +476,9 @@ unsealed runtimeclass TableView : Microsoft.UI.Xaml.Controls.Control // Queues a coalesced re-resolve of every realized cell; it does not run before this call // returns. Use it after attaching a handler to a TableView whose rows are already realized, or - // when the data behind the tooltips changes. Safe to call from a handler. + // when the data behind the tooltips changes. Calling it from a CellToolTipRequested handler is + // re-entrancy-safe, but call it only when the state behind the tooltips has actually changed: a + // handler that invalidates on every raise is dropped after a few passes. void InvalidateCellToolTips(); static Microsoft.UI.Xaml.DependencyProperty ItemsSourceProperty { get; }; diff --git a/controls/dev/TableView/TableViewRow.cpp b/controls/dev/TableView/TableViewRow.cpp index ec9aa262e4..d59b1189fa 100644 --- a/controls/dev/TableView/TableViewRow.cpp +++ b/controls/dev/TableView/TableViewRow.cpp @@ -17,6 +17,10 @@ static constexpr std::wstring_view s_CellsHostPartName{ L"PART_CellsHost"sv }; namespace { + // One pass, plus replays for app code that mutates the row from a CellToolTipRequested handler. + // Shared by both drain loops so a handler cannot get more replays by entering the other one. + constexpr int c_maxCellDrainPasses = 3; + constexpr winrt::Thickness s_verticalThickness{ 0, 0, 1, 0 }; constexpr winrt::Thickness s_zeroThickness{ 0, 0, 0, 0 }; @@ -286,10 +290,11 @@ void TableViewRow::OnDataContextChanged( { // On recycle ItemsRepeater updates the row's DataContext; cells pick up the new item reactively // via inheritance (they are not restamped). RebuildCells is still called so index-dependent visuals - // (alternating-row banding, frozen pinning) refresh for the new position. Guard against re-entry - // Guarded by RebuildCells, which owns the re-entry policy for both rebuilds and the tooltip - // pass; a bare guard here would drop the request instead of recording it. - RebuildCells();} + // (alternating-row banding, frozen pinning) refresh for the new position. RebuildCells owns the + // re-entry policy for both rebuilds and the tooltip pass; a bare guard here would drop the + // request instead of recording it. + RebuildCells(); +} void TableViewRow::OnColumnsVectorChanged( const winrt::IObservableVector& /*sender*/, @@ -557,23 +562,14 @@ void TableViewRow::RebuildCells() // One rebuild plus replays for app code that mutates the row from a handler. Beyond the cap the // app is livelocking the UI thread, so the request is dropped with a diagnostic. - constexpr int c_maxRebuildDrainPasses = 3; - for (int pass = 0; pass < c_maxRebuildDrainPasses; ++pass) + // The loop calls the *Core* primitives, never the public RefreshCellToolTips: that entry point + // drains too, so calling it here would start a nested drain with a fresh counter and recurse + // until the stack died instead of tripping the cap. + for (int pass = 0; pass < c_maxCellDrainPasses; ++pass) { m_rebuildCellsPending = false; RebuildCellsCore(); - - // Contained here: this runs from framework callbacks, where an escaping handler exception is - // a fail-fast rather than a recoverable error. - try - { - RefreshCellToolTips(); - } - catch (...) - { - TVDiag::LogRetailF(L"[TableViewRow] CellToolTipRequested handler threw (HRESULT 0x%08X). Continuing.", - static_cast(winrt::to_hresult())); - } + RefreshCellToolTipsGuarded(); if (!m_rebuildCellsPending) { @@ -584,7 +580,7 @@ void TableViewRow::RebuildCells() if (m_rebuildCellsPending) { TVDiag::LogRetailF(L"[TableViewRow] Dropping a pending cell rebuild after %d drain passes.", - c_maxRebuildDrainPasses); + c_maxCellDrainPasses); m_rebuildCellsPending = false; } } @@ -846,13 +842,54 @@ void TableViewRow::RebuildCellsCore() void TableViewRow::RefreshCellToolTips() { - RefreshCellToolTipsCore(); + // A handler can drop the last reference to this row. + auto strongThis = get_strong(); - // Only RebuildCells drains, so a refresh reached directly (invalidate, edit close) must drain the - // rebuild a handler queued, or the row is left stamped for the previous state. - if (m_rebuildCellsPending && !m_isRebuildingCells && !m_isRefreshingCellToolTips) + // A drain is already running (this is the rebuild loop's own tooltip pass, or a nested call from + // a handler). That loop owns the replay; adding one here is what used to recurse. + if (m_isRebuildingCells || m_isRefreshingCellToolTips) { - RebuildCells(); + RefreshCellToolTipsGuarded(); + return; + } + + // Reached directly (invalidate, edit close), so this is the drain owner: a rebuild a handler + // queued must be replayed here, or the row is left stamped for the previous state. Iterative + // over the *Core* primitives - calling RebuildCells would start a second drain and recurse. + for (int pass = 0; pass < c_maxCellDrainPasses; ++pass) + { + RefreshCellToolTipsGuarded(); + + if (!m_rebuildCellsPending) + { + return; + } + + m_rebuildCellsPending = false; + RebuildCellsCore(); + } + + if (m_rebuildCellsPending) + { + TVDiag::LogRetailF(L"[TableViewRow] Dropping a pending cell rebuild after %d drain passes.", + c_maxCellDrainPasses); + m_rebuildCellsPending = false; + } +} + +// Contained here because every caller runs from a framework callback (OnApplyTemplate, +// OnDataContextChanged, the repeater's measure pass, a dispatcher callback), where letting an app +// handler's exception cross back into XAML is a fail-fast rather than a recoverable error. +void TableViewRow::RefreshCellToolTipsGuarded() +{ + try + { + RefreshCellToolTipsCore(); + } + catch (...) + { + TVDiag::LogRetailF(L"[TableViewRow] CellToolTipRequested handler threw (HRESULT 0x%08X). Continuing.", + static_cast(winrt::to_hresult())); } } @@ -939,12 +976,14 @@ void TableViewRow::RefreshCellToolTipsCore() auto const dataItem = ownerImpl->UnwrapEditingDataItem(DataContext()); for (auto const& target : targets) { - auto const args = ownerImpl->RaiseCellToolTipRequested(target.first, dataItem); - auto const content = args->Content(); - try { - // Separate from the raise: app content may be already parented, which throws on assign. + // The raise is inside the try, not just the apply: a throwing handler must not escape + // into the framework callback this pass runs from. + auto const args = ownerImpl->RaiseCellToolTipRequested(target.first, dataItem); + auto const content = args->Content(); + + // App content may be already parented, which throws on assign. const bool owned = content ? TableViewDetails::SetOwnedToolTip( target.second, @@ -959,7 +998,7 @@ void TableViewRow::RefreshCellToolTipsCore() { // Assume the element still holds something: the row must revisit it rather than skip it. anyOwned = true; - TVDiag::LogRetailF(L"[TableViewRow] Applying CellToolTipRequested content failed (HRESULT 0x%08X).", + TVDiag::LogRetailF(L"[TableViewRow] Resolving a cell tooltip failed (HRESULT 0x%08X).", static_cast(winrt::to_hresult())); } } diff --git a/controls/dev/TableView/TableViewRow.h b/controls/dev/TableView/TableViewRow.h index 2a3f9d53dd..340d90e0d4 100644 --- a/controls/dev/TableView/TableViewRow.h +++ b/controls/dev/TableView/TableViewRow.h @@ -141,6 +141,8 @@ class TableViewRow : // The guarded body. RebuildCells wraps it so the tooltip pass runs with the guard released. void RebuildCellsCore(); void RefreshCellToolTipsCore(); + // Runs one tooltip pass with app-handler exceptions contained. + void RefreshCellToolTipsGuarded(); void ClearOwnedCellToolTips(const winrt::Panel& host); // Coalesces a burst of Columns-collection changes into a single cell rebuild on the next diff --git a/controls/dev/TableView/TableViewToolTipHelpers.h b/controls/dev/TableView/TableViewToolTipHelpers.h index e2a78d384d..15f734f100 100644 --- a/controls/dev/TableView/TableViewToolTipHelpers.h +++ b/controls/dev/TableView/TableViewToolTipHelpers.h @@ -187,12 +187,12 @@ namespace TableViewDetails } // Publish unless the cell already reports the text, so Narrator does not read it twice. The - // cell's UIA text is resolved here, not by the caller: it can allocate a peer, and cells with - // no tooltip never need it. + // cheap checks come first: GetCellValueTextFromWrapper can allocate an automation peer for + // non-text content, and that must not run for cells that will not publish anyway. auto const helpText = !helpTextOverride.empty() ? helpTextOverride : (text ? *text : winrt::hstring{}); if (!helpText.empty() && - helpText != GetCellValueTextFromWrapper(element) && - winrt::AutomationProperties::GetHelpText(element).empty()) + winrt::AutomationProperties::GetHelpText(element).empty() && + helpText != GetCellValueTextFromWrapper(element)) { winrt::AutomationProperties::SetHelpText(element, helpText); record->PublishedHelpText = helpText; From a15a20d030dda6a9695191f8bfa60a49fce251d3 Mon Sep 17 00:00:00 2001 From: Hitesh Kumar Date: Fri, 28 Aug 2026 13:16:59 +0530 Subject: [PATCH 4/5] Tabular: rename the cell tooltip help text and tighten the pass Rename CellToolTipRequestedEventArgs.ToolTipHelpText to AutomationHelpText. The value is published verbatim as the cell's UIA HelpText, and the old name read as help text belonging to the tooltip. Resolve the cell's UIA text without creating an automation peer on the tooltip path. The duplicate-suppression comparison ran on every realization and every invalidate, so a template column allocated a peer per cell with no UIA listener attached. Refresh tooltips once more when a drain runs out of passes, so cells rebuilt on the last pass do not keep the previous item's tooltips, and the drop diagnostic can be reached. Make the synchronous invalidate fallback terminal: it now drops a handler-requested follow-up with a diagnostic rather than silently, and resets the pass counter so a later invalidate is not capped early. Correct two IDL claims: the accessible text is suppressed when the cell already reports it, and the invalidate does run synchronously when there is no dispatcher. Document the tooltip accessibility contract in the API spec, record that the event is also raised when a cell edit closes, and state that content returned as a UIElement is parented by that cell's tooltip and cannot be shared across cells. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- controls/dev/TableView/TableView.cpp | 40 +++++++++------ controls/dev/TableView/TableView.h | 12 ++--- controls/dev/TableView/TableView.idl | 27 +++++----- .../TableView/TableViewAutomationHelpers.h | 11 ++-- .../TableViewCellToolTipRequestedEventArgs.h | 6 +-- controls/dev/TableView/TableViewRow.cpp | 51 +++++++++---------- controls/dev/TableView/TableViewRow.h | 17 +++---- .../dev/TableView/TableViewToolTipHelpers.h | 31 +++++------ docs/api-specs/TableView/TableView-spec.md | 16 ++++-- .../TableView-functional-spec.md | 10 ++-- 10 files changed, 121 insertions(+), 100 deletions(-) diff --git a/controls/dev/TableView/TableView.cpp b/controls/dev/TableView/TableView.cpp index ded08630a9..88e8c90593 100644 --- a/controls/dev/TableView/TableView.cpp +++ b/controls/dev/TableView/TableView.cpp @@ -1147,11 +1147,10 @@ winrt::com_ptr TableView::RaiseCellToolT void TableView::InvalidateCellToolTips() { - // Deferred: this is public, so an app can call it from its own handler. Raising synchronously - // would re-enter the pass and walk live children mid-mutation. Matches InvalidateItemsInfo. + // Deferred because this is public: raising synchronously from an app handler would re-enter the + // pass and walk live children mid-mutation. Matches InvalidateItemsInfo. if (m_cellToolTipRefreshQueued) { - // Already queued or running: coalesce into exactly one follow-up. m_cellToolTipRefreshDirty = true; return; } @@ -1159,8 +1158,6 @@ void TableView::InvalidateCellToolTips() auto dispatcher = DispatcherQueue(); if (!dispatcher) { - // No dispatcher to defer onto. The flag is still held across the synchronous pass so a - // handler calling back coalesces into a follow-up instead of re-entering and recursing. RefreshCellToolTipsSynchronously(); return; } @@ -1171,9 +1168,9 @@ void TableView::InvalidateCellToolTips() { if (auto strongThis = weakThis.get()) { - // The flag stays set across the pass, or a handler calling back would queue a - // callback every tick. Cleared on the throw path too: leaving it set would silently - // disable every future invalidate. + // Held across the pass so a handler calling back records a follow-up instead of + // queueing a callback every tick, and cleared on the throw path so a failed pass + // cannot disable every future invalidate. strongThis->m_cellToolTipRefreshDirty = false; try { @@ -1190,9 +1187,8 @@ void TableView::InvalidateCellToolTips() { strongThis->m_cellToolTipRefreshDirty = false; - // A handler that invalidates on every raise would otherwise re-arm this forever, - // pegging the UI thread with a dispatcher callback per tick. Bounded like the - // row's rebuild drain: beyond the cap the request is dropped with a diagnostic. + // A handler invalidating on every raise would re-arm this forever, so it is + // bounded like the row's rebuild drain. if (++strongThis->m_cellToolTipRefreshPasses < c_maxCellToolTipRefreshPasses) { strongThis->InvalidateCellToolTips(); @@ -1213,15 +1209,28 @@ void TableView::InvalidateCellToolTips() } } -// Runs a pass with the coalescing flag held, so a handler that calls InvalidateCellToolTips records -// a follow-up instead of re-entering this synchronously and recursing without bound. +// Holds the coalescing flag so a handler calling InvalidateCellToolTips records a follow-up instead +// of re-entering synchronously and recursing without bound. Terminal: there is no dispatcher to +// replay the follow-up on, so it is dropped with a diagnostic rather than silently. void TableView::RefreshCellToolTipsSynchronously() { m_cellToolTipRefreshQueued = true; - auto queuedGuard = wil::scope_exit([this]() { m_cellToolTipRefreshQueued = false; }); + auto queuedGuard = wil::scope_exit([this]() + { + m_cellToolTipRefreshQueued = false; + m_cellToolTipRefreshPasses = 0; + }); RefreshCellToolTipsOnRealizedRows(); + + if (m_cellToolTipRefreshDirty) + { + m_cellToolTipRefreshDirty = false; + TVDiag::LogRetailF(L"[TableView] Dropping a cell-tooltip re-resolve requested during a " + L"synchronous fallback pass; there is no dispatcher to replay it on."); + } } + void TableView::RefreshCellToolTipsOnRealizedRows() { ForEachRealizedRow([](winrt::TableViewRow const& row) @@ -1234,11 +1243,12 @@ void TableView::RefreshCellToolTipsOnRealizedRows() { // Contained per row: an exception escaping a dispatcher callback fail-fasts, and one // bad row must not abort the rest. - TVDiag::LogRetailF(L"[TableView] CellToolTipRequested handler threw (HRESULT 0x%08X). Continuing.", + TVDiag::LogRetailF(L"[TableView] A row's cell tooltip pass failed (HRESULT 0x%08X). Continuing.", static_cast(winrt::to_hresult())); } }); } + void TableView::OnRowElementClearing( const winrt::ItemsRepeater& /*sender*/, const winrt::ItemsRepeaterElementClearingEventArgs& args) diff --git a/controls/dev/TableView/TableView.h b/controls/dev/TableView/TableView.h index 5c13377088..3262cbc91e 100644 --- a/controls/dev/TableView/TableView.h +++ b/controls/dev/TableView/TableView.h @@ -141,8 +141,8 @@ class TableView : // Pin rebuilt rows immediately when leading-frozen columns are active. void PinFrozenColumnsForRow(const winrt::TableViewRow& row); - // Internal — true while at least one CellToolTipRequested handler is attached. Lets the cell - // realization path skip all per-cell tooltip work when nobody is listening. + // Internal — true while at least one CellToolTipRequested handler is attached, so the cell + // realization path can skip all per-cell tooltip work when nobody is listening. bool HasCellToolTipHandler() const { return static_cast(m_cellToolTipRequestedEventSource); } // Internal — raises CellToolTipRequested for one cell and returns the args the handler filled in. @@ -152,8 +152,6 @@ class TableView : // Queues a coalesced tooltip re-resolve for every realized cell (public, from the IDL). void InvalidateCellToolTips(); - void RefreshCellToolTipsOnRealizedRows(); - void RefreshCellToolTipsSynchronously(); // Requested by a cell panel (header/row) during measure when a realized cell's own measured width // changed (grow or shrink). Invalidates our measure synchronously so ResolveColumnWidths re-runs in @@ -572,12 +570,14 @@ class TableView : // Set while a coalesced RebuildHeaders is pending on the dispatcher; collapses a burst of column // changes into one rebuild. UI-thread only (all column callbacks arrive on the UI thread). bool m_rebuildHeadersQueued{ false }; + void RefreshCellToolTipsOnRealizedRows(); + void RefreshCellToolTipsSynchronously(); // A coalesced cell-tooltip re-resolve is pending on the dispatcher (InvalidateCellToolTips). bool m_cellToolTipRefreshQueued{ false }; // An invalidate arrived while a pass was queued or running; coalesced into one follow-up. bool m_cellToolTipRefreshDirty{ false }; - // Consecutive handler-requested follow-up passes, so an unconditionally-invalidating handler is - // dropped with a diagnostic instead of pegging the UI thread forever. + // Consecutive handler-requested follow-up passes, capped so an unconditionally-invalidating + // handler is dropped rather than pegging the UI thread. int m_cellToolTipRefreshPasses{ 0 }; // Per-instance resource cache; replaces the former process-global map keyed by `this`. diff --git a/controls/dev/TableView/TableView.idl b/controls/dev/TableView/TableView.idl index c40ba6cd74..5bb8d4434f 100644 --- a/controls/dev/TableView/TableView.idl +++ b/controls/dev/TableView/TableView.idl @@ -198,12 +198,14 @@ runtimeclass TableViewCellToolTipRequestedEventArgs // The tooltip content: a string, or any content a ToolTip can host. Null or an empty string // means no tooltip. A ToolTip is not valid content -- the control owns the ToolTip and its - // placement. + // placement. A UIElement is parented by that cell's ToolTip, so return a fresh element per + // raise: the same instance handed to a second cell cannot be parented twice and is dropped. Object Content { get; set; }; - // Accessible text, published as the cell's UIA HelpText. Set it when Content is not a string; - // it overrides string Content, and is ignored when Content is null. - String ToolTipHelpText { get; set; }; + // Accessible text, published as the cell's UIA HelpText unless the cell already reports that + // same text, so Narrator does not read it twice. Set it when Content is not a string; it + // overrides string Content, and is ignored when Content is null. + String AutomationHelpText { get; set; }; }; [MUX_PREVIEW] @@ -467,16 +469,17 @@ unsealed runtimeclass TableView : Microsoft.UI.Xaml.Controls.Control event Windows.Foundation.TypedEventHandler SelectionChanged; // ----- Tooltips ----- - // Raised as each cell's tooltip is resolved: on realization, on cell rebuild, and on - // InvalidateCellToolTips. May be raised repeatedly for the same cell, so handlers should be - // cheap and side-effect free. Opt-in -- not raised when no handler is attached. A tooltip the - // app sets inside a cell's own content template opens over that content; the control's tooltip - // covers the rest of the cell. + // Raised as each cell's tooltip is resolved: on realization, on cell rebuild, when a cell edit + // closes, and on InvalidateCellToolTips. May be raised repeatedly for the same cell, so handlers + // should be cheap and side-effect free. Opt-in -- not raised when no handler is attached. A + // tooltip the app sets inside a cell's own content template opens over that content; the + // control's tooltip covers the rest of the cell. event Windows.Foundation.TypedEventHandler CellToolTipRequested; - // Queues a coalesced re-resolve of every realized cell; it does not run before this call - // returns. Use it after attaching a handler to a TableView whose rows are already realized, or - // when the data behind the tooltips changes. Calling it from a CellToolTipRequested handler is + // Queues a coalesced re-resolve of every realized cell; it normally does not run before this + // call returns, falling back to running synchronously only when there is no dispatcher. Use it + // after attaching a handler to a TableView whose rows are already realized, or when the data + // behind the tooltips changes. Calling it from a CellToolTipRequested handler is // re-entrancy-safe, but call it only when the state behind the tooltips has actually changed: a // handler that invalidates on every raise is dropped after a few passes. void InvalidateCellToolTips(); diff --git a/controls/dev/TableView/TableViewAutomationHelpers.h b/controls/dev/TableView/TableViewAutomationHelpers.h index e19bc2819f..886faa736d 100644 --- a/controls/dev/TableView/TableViewAutomationHelpers.h +++ b/controls/dev/TableView/TableViewAutomationHelpers.h @@ -55,7 +55,9 @@ inline std::optional TryGetColumnHeaderString(winrt::TableViewCo // standard UIA name of the column-generated content. Shared by TableViewCellAutomationPeer (which // composes it into the cell name) and the cell tooltip pass (which uses it to avoid publishing // HelpText that would make Narrator read the same string twice), so the two cannot drift. -inline winrt::hstring GetCellValueTextFromWrapper(winrt::FrameworkElement const& cellWrapper) +// allowPeerCreation is false on the tooltip path: resolving a non-text cell allocates an automation +// peer, and that must not happen on the scroll path with no UIA listener. +inline winrt::hstring GetCellValueTextFromWrapper(winrt::FrameworkElement const& cellWrapper, bool allowPeerCreation = true) { if (!cellWrapper) { @@ -77,9 +79,12 @@ inline winrt::hstring GetCellValueTextFromWrapper(winrt::FrameworkElement const& return textBlock.Text(); } - if (auto const peer = winrt::FrameworkElementAutomationPeer::CreatePeerForElement(content)) + if (allowPeerCreation) { - return peer.GetName(); + if (auto const peer = winrt::FrameworkElementAutomationPeer::CreatePeerForElement(content)) + { + return peer.GetName(); + } } return {}; diff --git a/controls/dev/TableView/TableViewCellToolTipRequestedEventArgs.h b/controls/dev/TableView/TableViewCellToolTipRequestedEventArgs.h index 3e61cb1536..8ad58c4293 100644 --- a/controls/dev/TableView/TableViewCellToolTipRequestedEventArgs.h +++ b/controls/dev/TableView/TableViewCellToolTipRequestedEventArgs.h @@ -21,12 +21,12 @@ class TableViewCellToolTipRequestedEventArgs : winrt::TableViewColumn Column() { return m_column; } winrt::IInspectable Content() { return m_content; } void Content(winrt::IInspectable const& value) { m_content = value; } - winrt::hstring ToolTipHelpText() { return m_toolTipHelpText; } - void ToolTipHelpText(winrt::hstring const& value) { m_toolTipHelpText = value; } + winrt::hstring AutomationHelpText() { return m_automationHelpText; } + void AutomationHelpText(winrt::hstring const& value) { m_automationHelpText = value; } private: winrt::TableViewColumn m_column{ nullptr }; winrt::IInspectable m_item{ nullptr }; winrt::IInspectable m_content{ nullptr }; - winrt::hstring m_toolTipHelpText{}; + winrt::hstring m_automationHelpText{}; }; diff --git a/controls/dev/TableView/TableViewRow.cpp b/controls/dev/TableView/TableViewRow.cpp index d59b1189fa..a5ce9d58a4 100644 --- a/controls/dev/TableView/TableViewRow.cpp +++ b/controls/dev/TableView/TableViewRow.cpp @@ -550,8 +550,7 @@ void TableViewRow::InvalidateCells() void TableViewRow::RebuildCells() { - // The tooltip pass calls app code, so it runs outside the re-entry guard. A handler can drop the - // last reference to this row. + // A handler can drop the last reference to this row. auto strongThis = get_strong(); if (m_isRebuildingCells || m_isRefreshingCellToolTips) @@ -560,11 +559,9 @@ void TableViewRow::RebuildCells() return; } - // One rebuild plus replays for app code that mutates the row from a handler. Beyond the cap the - // app is livelocking the UI thread, so the request is dropped with a diagnostic. - // The loop calls the *Core* primitives, never the public RefreshCellToolTips: that entry point - // drains too, so calling it here would start a nested drain with a fresh counter and recurse - // until the stack died instead of tripping the cap. + // Replays a rebuild an app handler queued. Calls the Core primitives, never the public + // RefreshCellToolTips, which drains too and would nest a fresh counter instead of tripping + // the cap. Beyond it the app is livelocking the UI thread, so the request is dropped. for (int pass = 0; pass < c_maxCellDrainPasses; ++pass) { m_rebuildCellsPending = false; @@ -845,17 +842,16 @@ void TableViewRow::RefreshCellToolTips() // A handler can drop the last reference to this row. auto strongThis = get_strong(); - // A drain is already running (this is the rebuild loop's own tooltip pass, or a nested call from - // a handler). That loop owns the replay; adding one here is what used to recurse. + // A drain is already running and owns the replay. if (m_isRebuildingCells || m_isRefreshingCellToolTips) { RefreshCellToolTipsGuarded(); return; } - // Reached directly (invalidate, edit close), so this is the drain owner: a rebuild a handler - // queued must be replayed here, or the row is left stamped for the previous state. Iterative - // over the *Core* primitives - calling RebuildCells would start a second drain and recurse. + // Reached directly (invalidate, edit close), so this owns the drain: a rebuild a handler queued + // must be replayed here, or the row stays stamped for the previous state. Iterative over the + // Core primitives - calling RebuildCells would start a second drain and recurse. for (int pass = 0; pass < c_maxCellDrainPasses; ++pass) { RefreshCellToolTipsGuarded(); @@ -869,6 +865,10 @@ void TableViewRow::RefreshCellToolTips() RebuildCellsCore(); } + // Out of passes with cells just rebuilt: resolve their tooltips before giving up, or the row + // keeps the new cells with the previous item's tooltips. + RefreshCellToolTipsGuarded(); + if (m_rebuildCellsPending) { TVDiag::LogRetailF(L"[TableViewRow] Dropping a pending cell rebuild after %d drain passes.", @@ -877,9 +877,7 @@ void TableViewRow::RefreshCellToolTips() } } -// Contained here because every caller runs from a framework callback (OnApplyTemplate, -// OnDataContextChanged, the repeater's measure pass, a dispatcher callback), where letting an app -// handler's exception cross back into XAML is a fail-fast rather than a recoverable error. +// Callers run from framework callbacks, where an escaping handler exception is a fail-fast. void TableViewRow::RefreshCellToolTipsGuarded() { try @@ -888,14 +886,15 @@ void TableViewRow::RefreshCellToolTipsGuarded() } catch (...) { - TVDiag::LogRetailF(L"[TableViewRow] CellToolTipRequested handler threw (HRESULT 0x%08X). Continuing.", + TVDiag::LogRetailF(L"[TableViewRow] The cell tooltip pass failed (HRESULT 0x%08X). Continuing.", static_cast(winrt::to_hresult())); } } void TableViewRow::RefreshCellToolTipsCore() { - // Guarded here, not at the caller: this is what calls app code, and it has several entry points. + // Innermost guard: the drains also test this flag to decide ownership, but the pass must be + // non-re-entrant from every entry point because it is what calls app code. if (m_isRefreshingCellToolTips) { return; @@ -912,8 +911,7 @@ void TableViewRow::RefreshCellToolTipsCore() auto refreshGuard = wil::scope_exit([this]() { m_isRefreshingCellToolTips = false; }); auto const owner = GetOwningTableView(); - // No owner: the row keeps its cells for the next prepare, but an owned tooltip can hold - // app-supplied content, so release it rather than pinning it in the pool. + // No owner: an owned tooltip can hold app content, so release it rather than pin it in the pool. if (!owner) { ClearOwnedCellToolTips(host); @@ -932,8 +930,8 @@ void TableViewRow::RefreshCellToolTipsCore() return; } - // Snapshot before raising: a handler may mutate Columns or the cell children. The scratch vector - // is moved to a local so a nested pass cannot clear it mid-iteration; capacity is handed back. + // Snapshot before raising: a handler may mutate Columns or the cell children. Moved to a local + // so a nested pass cannot clear it mid-iteration; capacity is handed back. auto targets = std::move(m_toolTipTargets); targets.clear(); auto const children = host.Children(); @@ -962,8 +960,8 @@ void TableViewRow::RefreshCellToolTipsCore() } } - // Commit on every exit, including the throw path. A handler can throw before any cell is visited, - // so the flag stays conservatively true unless the pass completed. + // A handler can throw before any cell is visited, so the flag stays conservatively true unless + // the pass completed. bool anyOwned = false; bool passCompleted = false; auto passGuard = wil::scope_exit([&]() @@ -976,19 +974,18 @@ void TableViewRow::RefreshCellToolTipsCore() auto const dataItem = ownerImpl->UnwrapEditingDataItem(DataContext()); for (auto const& target : targets) { + // Covers the raise as well as the apply: a throwing handler must not escape into the + // framework callback this pass runs from, and app content may already be parented. try { - // The raise is inside the try, not just the apply: a throwing handler must not escape - // into the framework callback this pass runs from. auto const args = ownerImpl->RaiseCellToolTipRequested(target.first, dataItem); auto const content = args->Content(); - // App content may be already parented, which throws on assign. const bool owned = content ? TableViewDetails::SetOwnedToolTip( target.second, content, - args->ToolTipHelpText(), + args->AutomationHelpText(), winrt::PlacementMode::Mouse) : (TableViewDetails::ClearOwnedToolTip(target.second), false); diff --git a/controls/dev/TableView/TableViewRow.h b/controls/dev/TableView/TableViewRow.h index 340d90e0d4..aafdf298e8 100644 --- a/controls/dev/TableView/TableViewRow.h +++ b/controls/dev/TableView/TableViewRow.h @@ -59,8 +59,7 @@ class TableViewRow : // Rebuild realized cells when column content changes at runtime. void RefreshCells(); - // Internal: re-resolves this row's cell tooltips against its current data item. Raised outside - // the cell-rebuild re-entry guard because it calls into app code. + // Internal: re-resolves this row's cell tooltips against its current data item. void RefreshCellToolTips(); // Releases control-owned cell tooltips (recycle-out). void ReleaseCellToolTips(); @@ -138,10 +137,9 @@ class TableViewRow : const winrt::Microsoft::UI::Xaml::DependencyPropertyChangedEventArgs& args); void RebuildCells(); - // The guarded body. RebuildCells wraps it so the tooltip pass runs with the guard released. + // The guarded bodies. RebuildCells and RefreshCellToolTips drain over these. void RebuildCellsCore(); void RefreshCellToolTipsCore(); - // Runs one tooltip pass with app-handler exceptions contained. void RefreshCellToolTipsGuarded(); void ClearOwnedCellToolTips(const winrt::Panel& host); @@ -170,15 +168,16 @@ class TableViewRow : // Prevent DataContextChanged re-entry while RebuildCells updates child DCs. bool m_isRebuildingCells{ false }; - // Set while the tooltip pass is running, so a nested RebuildCells from a handler rebuilds - // without starting another raise. + // Set while the tooltip pass is running, so a nested RebuildCells records instead of raising. bool m_isRefreshingCellToolTips{ false }; // A rebuild requested while the guard was held; replayed once the guard drops. bool m_rebuildCellsPending{ false }; - // Whether any cell on this row currently carries a control-created tooltip. Lets the pass stay - // free when the feature is unused, while still retracting tooltips after the last handler goes. + // Whether any cell on this row carries a control-created tooltip, so the pass stays free when + // the feature is unused but still retracts tooltips after the last handler goes. bool m_hasOwnedCellToolTips{ false }; - // Reused per pass so the virtualized path does not allocate on every recycle. + // Reused per pass so the virtualized path does not allocate on every recycle. Plain refs rather + // than tracker_ref: strictly stack-scoped, cleared by the pass guard on every path including + // throw, so it never outlives the pass. std::vector> m_toolTipTargets{}; // Set while a coalesced RebuildCells is pending on the dispatcher (Columns-vector-changed burst). diff --git a/controls/dev/TableView/TableViewToolTipHelpers.h b/controls/dev/TableView/TableViewToolTipHelpers.h index 15f734f100..3bf8e1143f 100644 --- a/controls/dev/TableView/TableViewToolTipHelpers.h +++ b/controls/dev/TableView/TableViewToolTipHelpers.h @@ -10,16 +10,14 @@ namespace TableViewDetails { - // The tooltip the control attached to an element, and the exact HelpText it published for it. - // Both are recorded: the published text is not always the tooltip's content, so re-deriving it - // would strand stale HelpText on a recycled cell. + // The tooltip the control attached, and the exact HelpText it published. The published text is + // not always the tooltip's content, so it is recorded rather than re-derived. struct CellToolTipRecord : winrt::implements { winrt::ToolTip ToolTip{ nullptr }; winrt::hstring PublishedHelpText{}; }; - // Not in the IDL: an app can reach the ToolTip's Tag, but not this. inline GlobalDependencyProperty s_cellToolTipRecordProperty{ nullptr }; inline winrt::DependencyProperty EnsureCellToolTipRecordProperty() @@ -38,7 +36,7 @@ namespace TableViewDetails return s_cellToolTipRecordProperty; } - // Called from the generated ClearTypeProperties, so a XAML re-init re-registers against the new core. + // Called from the generated ClearTypeProperties so a XAML re-init re-registers against the new core. inline void ClearCellToolTipProperties() { s_cellToolTipRecordProperty = nullptr; @@ -84,8 +82,8 @@ namespace TableViewDetails record.PublishedHelpText = {}; } - // The ToolTip stays attached and is neutralized rather than detached, so a recycled cell reuses - // it instead of allocating a Control per cell per scroll. Matches TabViewItem. + // Neutralized in place rather than detached, so a recycled cell reuses the ToolTip instead of + // allocating a Control per cell per scroll. Matches TabViewItem. inline void ClearOwnedToolTip(const winrt::FrameworkElement& element) { auto const record = GetRecord(element); @@ -98,8 +96,8 @@ namespace TableViewDetails if (record->ToolTip && record->ToolTip == winrt::ToolTipService::GetToolTip(element).try_as()) { - // Disable and close before dropping content: clearing an open tooltip's content removes a - // live popup's child, the shape behind the reentrant CPopup::RemoveChild crash. + // Close before dropping content: clearing an open tooltip's content removes a live + // popup's child, the shape behind the reentrant CPopup::RemoveChild crash. record->ToolTip.IsEnabled(false); if (record->ToolTip.IsOpen()) { @@ -109,13 +107,12 @@ namespace TableViewDetails } else { - // The app replaced or removed it; leave what is there now alone. ForgetRecord(element); } } - // Applies app-supplied content to a control-created cell wrapper. Returns whether the element is - // left carrying a control-owned tooltip. Never touches a tooltip the app set itself. + // Applies app-supplied content to a control-created cell wrapper, returning whether the element + // is left carrying a control-owned tooltip. Never touches a tooltip the app set itself. inline bool SetOwnedToolTip( const winrt::FrameworkElement& element, const winrt::IInspectable& content, @@ -129,7 +126,7 @@ namespace TableViewDetails auto record = GetRecord(element); // The raw value, not a ToolTip-narrowed one: ToolTipService stores whatever the app set, and - // the common form is a bare string. Narrowing first would read that as an empty slot. + // a bare string would otherwise read as an empty slot. auto const existingValue = winrt::ToolTipService::GetToolTip(element); auto const owned = (record && record->ToolTip && record->ToolTip == existingValue.try_as()) ? record->ToolTip : nullptr; @@ -180,19 +177,17 @@ namespace TableViewDetails toolTip.Content(content); toolTip.Placement(placement); - // Record before attaching: a throw then reads as "not ours" rather than orphaning a + // Recorded before attaching: a throw then reads as "not ours" rather than orphaning a // tooltip nothing can clear. record->ToolTip = toolTip; winrt::ToolTipService::SetToolTip(element, toolTip); } - // Publish unless the cell already reports the text, so Narrator does not read it twice. The - // cheap checks come first: GetCellValueTextFromWrapper can allocate an automation peer for - // non-text content, and that must not run for cells that will not publish anyway. + // Suppressed when the cell already reports the text, so Narrator does not read it twice. auto const helpText = !helpTextOverride.empty() ? helpTextOverride : (text ? *text : winrt::hstring{}); if (!helpText.empty() && winrt::AutomationProperties::GetHelpText(element).empty() && - helpText != GetCellValueTextFromWrapper(element)) + helpText != GetCellValueTextFromWrapper(element, false /* allowPeerCreation */)) { winrt::AutomationProperties::SetHelpText(element, helpText); record->PublishedHelpText = helpText; diff --git a/docs/api-specs/TableView/TableView-spec.md b/docs/api-specs/TableView/TableView-spec.md index 3ead33a146..e83455767a 100644 --- a/docs/api-specs/TableView/TableView-spec.md +++ b/docs/api-specs/TableView/TableView-spec.md @@ -445,7 +445,7 @@ Named to match `ItemsView`, which ships `Select` / `Deselect` / `IsSelected` ove | Event | Args | Description | |---|---|---| -| `CellToolTipRequested` | `TableViewCellToolTipRequestedEventArgs` | Raised when a cell's tooltip is resolved — on realization, on cell rebuild, and on `InvalidateCellToolTips()`. May be raised repeatedly for the same cell and item, so handlers should be cheap and side-effect free. Opt-in: with no handler attached the control does no per-cell work. `InvalidateCellToolTips()` queues a coalesced re-resolve for every realized cell, for handlers attached after realization or when the underlying data changes. | +| `CellToolTipRequested` | `TableViewCellToolTipRequestedEventArgs` | Raised when a cell's tooltip is resolved — on realization, on cell rebuild, when a cell edit closes, and on `InvalidateCellToolTips()`. May be raised repeatedly for the same cell and item, so handlers should be cheap and side-effect free. Opt-in: with no handler attached the control does no per-cell work. `InvalidateCellToolTips()` queues a coalesced re-resolve for every realized cell, for handlers attached after realization or when the underlying data changes; it is re-entrancy-safe, but a handler that invalidates on every raise is dropped after a few passes. | | `SelectionChanged` | `SelectionChangedEventArgs` | Raised after the selection has settled. Reading `SelectedItem`, `SelectedIndex`, or a row's `IsSelected` inside the handler observes the new state. Replacing a selection raises **one** event carrying both the removed and the added item. | Selection is raised only for real changes: a re-select of the already-selected row, or an index shift caused by a collection reshape, does not raise `SelectionChanged`. @@ -648,7 +648,17 @@ How an edit is being closed. |---|---| | `TableViewBeginningEditEventArgs` | `Item`, `Column` (read-only); `Cancel` (settable) | | `TableViewCellEditEndingEventArgs` | `Item`, `Column`, `EditAction` (read-only); `Cancel` (settable) | -| `TableViewCellToolTipRequestedEventArgs` | `Item`, `Column` (read-only); `Content`, `ToolTipHelpText` (settable) | +| `TableViewCellToolTipRequestedEventArgs` | `Item`, `Column` (read-only); `Content`, `AutomationHelpText` (settable) | + +### Cell tooltip accessibility + +The control owns the `ToolTip`; `Content` is its content, not a `ToolTip` to attach. A `UIElement` returned as `Content` is parented by that cell's `ToolTip`, so return a string or a fresh element per raise — the same instance handed to a second cell cannot be parented twice and is dropped. + +- The tooltip text is published as the cell's `AutomationProperties.HelpText`, and retracted on recycle and when a cell edit begins. +- Publication is suppressed when the text equals the cell's own UIA text, so Narrator does not read it twice. +- The popup is **pointer-only**: cell focus in `TableView` is row-level, so there is no cell element for the framework's keyboard-tooltip path to fire on. The UIA pairing is what serves keyboard and screen-reader users, which is why it is not optional. +- Placement is control-owned and fixed (`PlacementMode.Mouse`), matching `TabViewItem`. An app needing different placement uses a tooltip inside its own cell content template. +- Setting `AutomationHelpText` is **required** when `Content` is not a string: non-string content cannot be stringified, and the cell wrapper the tooltip attaches to is internal, so an app cannot set `HelpText` on it. ## Selection event args @@ -738,7 +748,7 @@ namespace Microsoft.UI.Xaml.Controls.Tabular Object Item { get; }; Microsoft.UI.Xaml.Controls.Tabular.TableViewColumn Column { get; }; Object Content { get; set; }; - String ToolTipHelpText { get; set; }; + String AutomationHelpText { get; set; }; }; [MUX_PREVIEW, webhosthidden] diff --git a/docs/design-notes/TabularControls/TableView-functional-spec.md b/docs/design-notes/TabularControls/TableView-functional-spec.md index 6460f1fefa..1fcb29794f 100644 --- a/docs/design-notes/TabularControls/TableView-functional-spec.md +++ b/docs/design-notes/TabularControls/TableView-functional-spec.md @@ -76,11 +76,13 @@ Each item notes the MLP/v1 vs deferred status and the delivering PR. - Cell-level styling: custom cells via `TableViewTemplateColumn` + built-in text-cell defaults (left, vertically centered). A public per-column alignment/weight API — **deferred (PR3+)**. ### Tooltips -- Cell: opt-in via `CellToolTipRequested`; the app supplies content per cell on demand. No handler means no tooltip and no per-cell cost. Needed because text cells render with `CharacterEllipsis` and no wrapping, so over-wide values are otherwise unreadable. +- Cell: opt-in via `CellToolTipRequested`; the app supplies content per cell on demand. No handler means no tooltip and no per-cell cost. Needed because text cells render with `CharacterEllipsis` and no wrapping, so over-wide values are otherwise unreadable. — **PR3** +- Raised as each cell's tooltip is resolved: on realization, on cell rebuild, when a cell edit closes, and on `InvalidateCellToolTips()`. - Author precedence: a tooltip set inside a cell's own content template opens over that content; the control's tooltip covers the rest of the cell, and the control never touches a tooltip it did not attach. -- Accessibility: the tooltip text is published as the cell's `AutomationProperties.HelpText` unless the cell already reports it, so the information is not pointer-only and is not announced twice. For content UIA cannot read the handler sets `ToolTipHelpText`, since the cell wrapper is not reachable from the app. -- Recycling: a recycled row never shows a previous item's cell tooltip, including after the last handler is removed. `InvalidateCellToolTips()` queues a coalesced re-resolve of every realized cell when a handler is attached late or the underlying data changes. -- Column-header and group-header tooltips are **deferred**: the header band is `IsHitTestVisible="False"` (`TableView.xaml`) so a header tooltip could never open, and grouping is not wired up on `main`. Both land with the work that makes those surfaces interactive. +- Content: a string, or any content a `ToolTip` can host. A `UIElement` is parented by that cell's `ToolTip`, so a handler returns a fresh element per raise — the same instance handed to a second cell cannot be parented twice and is dropped. +- Accessibility: the tooltip text is published as the cell's `AutomationProperties.HelpText` unless the cell already reports it, so the information is not pointer-only and is not announced twice. For content UIA cannot read the handler sets `AutomationHelpText`, since the cell wrapper is not reachable from the app. +- Recycling: a recycled row never shows a previous item's cell tooltip, including after the last handler is removed. `InvalidateCellToolTips()` queues a coalesced re-resolve of every realized cell when a handler is attached late or the underlying data changes. It is re-entrancy-safe, but a handler that invalidates on every raise is dropped after a few passes. +- Column-header and group-header tooltips are **deferred**: the header band is `IsHitTestVisible="False"` (`TableView.xaml`) so a header tooltip could never open, and grouping is not yet enabled. Both land with the work that makes those surfaces interactive. > Tooltips are **not** gated on text truncation. No WinUI control keys tooltips off `IsTextTrimmed`; the shipped pattern is to gate on a cheap content predicate (non-empty string) or an explicit opt-in. From 5e6305887d312875009482994876b8d15bbf5b0a Mon Sep 17 00:00:00 2001 From: Hitesh Kumar Date: Fri, 28 Aug 2026 13:44:40 +0530 Subject: [PATCH 5/5] Tabular: report a pointer-only cell tooltip, and mark the codegen hand-edits Non-string tooltip content with no AutomationHelpText reaches assistive technology through nothing but the popup. The control does not synthesize the text - no XAML control derives HelpText from a content peer, and a panel's peer name is empty in the common case - so it reports the condition instead of failing silently. Logged once per pass rather than per cell, since a whole column would otherwise log on every scroll. Mark the two hand-edited lines in the Tabular metadata provider template, which a future sync from the MUXC template would otherwise drop silently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- controls/dev/TableView/TableViewRow.cpp | 12 ++++++++++++ .../dll-tabular/XamlMetadataProviderGenerated.tt | 15 ++++++++------- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/controls/dev/TableView/TableViewRow.cpp b/controls/dev/TableView/TableViewRow.cpp index a5ce9d58a4..bbf027c0a8 100644 --- a/controls/dev/TableView/TableViewRow.cpp +++ b/controls/dev/TableView/TableViewRow.cpp @@ -972,6 +972,7 @@ void TableViewRow::RefreshCellToolTipsCore() }); auto const dataItem = ownerImpl->UnwrapEditingDataItem(DataContext()); + bool anyPointerOnly = false; for (auto const& target : targets) { // Covers the raise as well as the apply: a throwing handler must not escape into the @@ -981,6 +982,11 @@ void TableViewRow::RefreshCellToolTipsCore() auto const args = ownerImpl->RaiseCellToolTipRequested(target.first, dataItem); auto const content = args->Content(); + // Non-string content with no accessible text is pointer-only. Reported once per pass + // rather than per cell, since a whole column would otherwise log on every scroll. + anyPointerOnly = anyPointerOnly || + (content && args->AutomationHelpText().empty() && !TableViewDetails::TryGetString(content)); + const bool owned = content ? TableViewDetails::SetOwnedToolTip( target.second, @@ -1000,6 +1006,12 @@ void TableViewRow::RefreshCellToolTipsCore() } } + if (anyPointerOnly) + { + TVDiag::LogRetailF(L"[TableViewRow] A cell tooltip is pointer-only: its content is not text and " + L"CellToolTipRequestedEventArgs.AutomationHelpText was not set."); + } + passCompleted = true; } diff --git a/controls/dev/dll-tabular/XamlMetadataProviderGenerated.tt b/controls/dev/dll-tabular/XamlMetadataProviderGenerated.tt index 460a594a86..30e46b123e 100644 --- a/controls/dev/dll-tabular/XamlMetadataProviderGenerated.tt +++ b/controls/dev/dll-tabular/XamlMetadataProviderGenerated.tt @@ -206,11 +206,12 @@ struct Entry WriteLine("#include \"" + type.Name + ".properties.h\""); } - // TableView's cell-tooltip ownership property is registered lazily outside the generated - // TableViewProperties, so the generated file owns its include as well as the clear call below -- - // otherwise the generated header would only compile when its includer happened to add it first. + // HAND-EDIT (keep when syncing this template): TableView's cell-tooltip ownership property is + // registered lazily outside the generated TableViewProperties, so the generated file owns its + // include as well as the clear call below -- otherwise the generated header would only compile + // when its includer happened to add it first. WriteLine("#include \"TableViewToolTipHelpers.h\""); - + // The Tabular controls DLL hosts only TableView + its primitives; the MUXC-only helpers // (AutoSuggestBoxHelper / ComboBoxHelper / RecyclePool / RevealBrush) that the full MUXC // provider manually includes do not exist in this binary, so they are intentionally omitted. @@ -230,9 +231,9 @@ namespace { // The Tabular controls DLL hosts only TableView + its primitives; the MUXC-only helpers // (AutoSuggestBoxHelper / ComboBoxHelper / RecyclePool / RevealBrush) are intentionally omitted. // - // TableView's cell-tooltip ownership property is registered lazily outside the generated - // TableViewProperties, so it needs clearing here too or a XAML re-init would keep handing - // out a registration from the dead core. + // HAND-EDIT (keep when syncing this template): TableView's cell-tooltip ownership property + // is registered lazily outside the generated TableViewProperties, so it needs clearing here + // too or a XAML re-init would keep handing out a registration from the dead core. WriteLine("TableViewDetails::ClearCellToolTipProperties();"); }); WriteLine("");