From 5cb91502b35d51d80e76a4a4bf4a7f95f4a26011 Mon Sep 17 00:00:00 2001 From: Dmitry Ratner <6830384+dmitrat@users.noreply.github.com> Date: Wed, 19 Aug 2026 12:49:22 +0300 Subject: [PATCH] The double click was repaired onto a route that does not exist Reported: a double click on a table opens nothing, it only opens the row. It has been that way since the repair that was supposed to fix it, and it shipped in 3.1.0. DoubleTapped is registered Bubble ALONE. Asking for its tunnelling route is not asking for an early handler, it is asking for no handler: AddHandler takes the route as a plain argument and cannot refuse an impossible one, so the call compiled, ran, registered nothing, and said nothing. From that commit the double click did nothing at all - through 1014 green tests, a green CI and a signed release. The pointer is the event that tunnels, so the double click is read from it: PointerPressed with ClickCount == 2, before the row sees the press. A press on the chevron is left alone - two clicks there are two toggles. Handling the press does NOT stop the tap. The gesture is recognised from the finished route, so the row still toggled itself and a table both opened its data and opened its row - measured, not reasoned. A second handler, bubbling and handledEventsToo, puts back the row a double click has just opened the data of. The chevron and the arrow keys are untouched. The guard is the general rule rather than this instance: every AddHandler in Studio is read out of the source, the event resolved to the real RoutedEvent, and the route asked for must be one the event actually travels. Red on the shipped code, red again with a sabotaged route, and it names what it read so that a walk which found nothing cannot pass. Measured against a fresh build: a table opens its data and stays closed, a view opens its top 1000 and stays closed, the chevron still opens the columns. Studio: 1021 green. Co-Authored-By: Claude Opus 5 --- ...lerRunsOnlyOnARouteItsEventTravelsTests.cs | 209 ++++++++++++++++++ .../Views/DatabaseExplorer.axaml.cs | 135 ++++++++--- 2 files changed, 319 insertions(+), 25 deletions(-) create mode 100644 Tools/OutWit.Database.Studio.Tests/AHandlerRunsOnlyOnARouteItsEventTravelsTests.cs diff --git a/Tools/OutWit.Database.Studio.Tests/AHandlerRunsOnlyOnARouteItsEventTravelsTests.cs b/Tools/OutWit.Database.Studio.Tests/AHandlerRunsOnlyOnARouteItsEventTravelsTests.cs new file mode 100644 index 0000000..7aa0bd7 --- /dev/null +++ b/Tools/OutWit.Database.Studio.Tests/AHandlerRunsOnlyOnARouteItsEventTravelsTests.cs @@ -0,0 +1,209 @@ +using System.Text.RegularExpressions; +using Avalonia.Controls; +using Avalonia.Input; +using Avalonia.Interactivity; + +namespace OutWit.Database.Studio.Tests; + +/// +/// A handler asked for a route its event does not travel is never called, and nothing says so. +/// +/// +/// +/// Measured on 2026-08-19, and it had shipped. The double click that opens a table's data +/// (WS-19) stopped working once a table had a placeholder child to expand, because the tree toggles +/// the row on a double tap and marks the event handled. The repair was to register the handler as +/// TUNNELLING, so that it would run first - and DoubleTapped is registered as +/// Bubble ALONE. A tunnelling handler for it is not an early handler, it is no handler: the +/// double click did nothing at all from that commit until this one, and 1014 tests, a green CI and a +/// signed release had nothing to say about it. +/// +/// +/// AddHandler takes the route as a plain argument and cannot refuse an impossible one, so this +/// is checked here: every AddHandler in Studio is read out of the source, the event is +/// resolved to the real , and the route asked for must be one the event +/// actually travels. +/// +/// +[TestFixture] +public class AHandlerRunsOnlyOnARouteItsEventTravelsTests +{ + #region Constants + + /// + /// AddHandler(SomeEvent, Handler, ...), whether or not it names a route. A call that + /// leaves the route out gets Bubble, which every routed event travels; those are counted anyway, + /// because a walk that reports no offenders has to be able to say what it read. + /// + private static readonly Regex ADD_HANDLER = new( + @"AddHandler\(\s*(?[\w\.]+)\s*,", + RegexOptions.Compiled); + + /// The route asked for, inside the same statement. + private static readonly Regex ROUTES = new( + @"RoutingStrategies\.(?[\w\s\|\.]+?)\s*[,\)]", + RegexOptions.Compiled); + + #endregion + + #region Tests + + [Test] + public void EveryHandlerInStudioAsksForARouteItsEventTravelsTest() + { + var events = TheRoutedEventsAvaloniaPublishes(); + + var offenders = new List(); + var examined = new List(); + var asked = 0; + + foreach (var (file, source) in StudioSources()) + foreach (Match match in ADD_HANDLER.Matches(source)) + { + var name = match.Groups["event"].Value.Split('.')[^1]; + + Assert.That(events.ContainsKey(name), Is.True, + $"{file}: this fixture could not resolve {name} to a routed event"); + + examined.Add($"{file}: {name}"); + + // The overload without a route gives the handler Bubble, which every routed event + // travels. Only a call that NAMES one can name an impossible one. + var written = ROUTES.Match(Statement(source, match.Index)); + + if (!written.Success) + continue; + + asked++; + + var routes = Routes(written.Groups["routes"].Value); + var travels = events[name].RoutingStrategies; + + if ((routes & travels) != routes) + offenders.Add($"{file}: {name} travels {travels}, and the handler asks for {routes} - " + + "the part that is not there is never called"); + } + + Assert.Multiple(() => + { + // CONTROL: a walk that read no source, or a pattern that matched nothing, would report + // no offenders either - which is exactly what this fixture is here to disbelieve. Both + // halves of the reader are named: the calls it found, and the routes it read out of + // them. + Assert.That(examined, Has.Count.GreaterThanOrEqualTo(2), + "CONTROL: too few AddHandler calls were found - the walk or the pattern is wrong"); + + Assert.That(asked, Is.GreaterThanOrEqualTo(1), + "CONTROL: no route was read out of any of them - " + string.Join(", ", examined)); + + Assert.That(offenders, Is.Empty, string.Join(Environment.NewLine, offenders)); + }); + } + + /// + /// The fact the case above was written for, stated on its own: DoubleTapped bubbles and + /// does not tunnel. + /// + /// + /// It is asserted rather than remembered because the repair that failed was reasoned from the + /// opposite assumption. If a later Avalonia gives the event a tunnelling route, this case is + /// where that shows up, and the rule above quietly starts allowing what it forbids today. + /// + [Test] + public void TheDoubleTapIsABubblingEventAndNothingElseTest() + { + Assert.Multiple(() => + { + Assert.That(InputElement.DoubleTappedEvent.RoutingStrategies, + Is.EqualTo(RoutingStrategies.Bubble), + "a tunnelling handler for the double tap is not an early handler, it is no handler"); + + // The pointer is the route that DOES have a tunnel, which is where the double click + // belongs once the tree wants to handle it before the row does. + Assert.That(InputElement.PointerPressedEvent.RoutingStrategies.HasFlag(RoutingStrategies.Tunnel), + Is.True, "the pointer tunnels, which is why the double click is read from it"); + }); + } + + #endregion + + #region Tools + + /// The statement the call is part of: from the call to the semicolon that ends it. + private static string Statement(string source, int index) + { + var end = source.IndexOf(';', index); + + return end < 0 ? source[index..] : source[index..end]; + } + + private static RoutingStrategies Routes(string written) + { + var routes = RoutingStrategies.Direct & 0; + + foreach (var part in written.Split('|', StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries)) + routes |= Enum.Parse(part.Split('.')[^1]); + + return routes; + } + + /// + /// Every routed event Avalonia publishes as a public static field, by name. + /// + private static IReadOnlyDictionary TheRoutedEventsAvaloniaPublishes() + { + var assemblies = new[] + { + typeof(InputElement).Assembly, + typeof(Control).Assembly, + typeof(RoutedEvent).Assembly + }.Distinct(); + + var events = new Dictionary(); + + foreach (var type in assemblies.SelectMany(assembly => assembly.GetExportedTypes())) + foreach (var field in type.GetFields(System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Static)) + { + if (!typeof(RoutedEvent).IsAssignableFrom(field.FieldType)) + continue; + + if (field.GetValue(null) is RoutedEvent value) + events.TryAdd(field.Name, value); + } + + return events; + } + + private static IEnumerable<(string File, string Source)> StudioSources() + { + var root = StudioRoot(); + + foreach (var file in Directory.EnumerateFiles(root, "*.cs", SearchOption.AllDirectories)) + { + if (file.Contains($"{Path.DirectorySeparatorChar}bin{Path.DirectorySeparatorChar}") + || file.Contains($"{Path.DirectorySeparatorChar}obj{Path.DirectorySeparatorChar}")) + continue; + + yield return (Path.GetRelativePath(root, file), File.ReadAllText(file)); + } + } + + private static string StudioRoot() + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + + while (directory != null) + { + var candidate = Path.Combine(directory.FullName, "Tools", "OutWit.Database.Studio"); + + if (Directory.Exists(Path.Combine(candidate, "Views"))) + return candidate; + + directory = directory.Parent; + } + + throw new AssertionException("the Studio project was not found from " + AppContext.BaseDirectory); + } + + #endregion +} diff --git a/Tools/OutWit.Database.Studio/Views/DatabaseExplorer.axaml.cs b/Tools/OutWit.Database.Studio/Views/DatabaseExplorer.axaml.cs index 6cbd6aa..f93d3fd 100644 --- a/Tools/OutWit.Database.Studio/Views/DatabaseExplorer.axaml.cs +++ b/Tools/OutWit.Database.Studio/Views/DatabaseExplorer.axaml.cs @@ -25,6 +25,10 @@ public partial class DatabaseExplorer : UserControl private string m_typed = string.Empty; private DateTime m_typedAt = DateTime.MinValue; + /// The row a double click has just opened the data of, and the state it was in. + private TreeViewItem? m_rowToLeaveAsItWas; + private bool m_asItWas; + #endregion #region Static @@ -63,17 +67,30 @@ public DatabaseExplorer() InitializeComponent(); DataContext = ApplicationViewModel.Instance; - // TUNNELLING since 2026-08-19. A TreeViewItem toggles its own expansion on a double - // click, and a table has had a child to expand since the placeholder arrived - so the - // bubbling handler ran after the row had opened, and opening the DATA (WS-19) stopped - // happening. Measured in the running application. - AddHandler(DoubleTappedEvent, OnDoubleTapped, RoutingStrategies.Tunnel); KeyDown += OnKeyDown; - // Tunnelling, because a TreeViewItem handles the pointer for its own selection and a - // bubbling handler would never see the middle button. + // Tunnelling, and it carries BOTH the middle click and the double click: a TreeViewItem + // handles the pointer for its own selection and for its own expansion, and a bubbling + // handler sees neither. + // + // The double click USED to be read from DoubleTapped, which is where it belongs - until a + // table gained a placeholder child, the row began toggling on a double tap, and the tree + // marked the event handled before this control saw it. That was repaired by asking for the + // TUNNELLING route of DoubleTapped, and there is no such route: the event is registered + // Bubble alone, so the handler was never called again and opening a table's data did + // nothing at all - through a green suite, a green CI and a signed release. The pointer is + // the event that tunnels, so the double click is read from it. See + // AHandlerRunsOnlyOnARouteItsEventTravelsTests. AddHandler(PointerPressedEvent, OnPointerPressed, RoutingStrategies.Tunnel); + // And the second half of the same click. Handling the pointer press does NOT stop the + // double tap: the gesture is recognised from the finished route, so the row still toggled + // itself and a table both opened its data and opened its row. Measured on 2026-08-19. This + // runs after the row has done that - bubbling, and handledEventsToo because the row marks + // the tap handled - and puts the row back the way it was. + AddHandler(DoubleTappedEvent, OnDoubleTappedAfterTheRow, RoutingStrategies.Bubble, + handledEventsToo: true); + // BUBBLING, and deliberately not tunnelling: the filter box and the rename box are text // boxes, and a tunnelling handler would eat every letter typed into them and jump the tree // instead. A box that has taken the character marks the event handled, and a bubbling @@ -99,26 +116,60 @@ public DatabaseExplorer() /// application's behaviour. /// /// - private void OnDoubleTapped(object? sender, Avalonia.Input.TappedEventArgs e) + private void OpenTheDataUnderThePointer(PointerPressedEventArgs e) { - var explorer = ApplicationViewModel.Instance.DatabaseExplorerVm; - var node = explorer.SelectedNode; + // The chevron is a control of its own, and two clicks on it are two toggles rather than a + // request for the data. + if (PressedOnTheChevron(e)) + return; - if (node == null) + if (ItemUnder(e) is not { } item || item.DataContext is not DatabaseNode node) return; + var explorer = ApplicationViewModel.Instance.DatabaseExplorerVm; + + // The row under the pointer, not the row that happens to be selected. The first click of + // the pair selects it anyway; saying so here is what makes the handler independent of the + // order the tree does its own work in. + explorer.SelectedNode = node; + switch (node.NodeType) { - case Models.DatabaseNodeType.Table when explorer.CanEditData: + case DatabaseNodeType.Table when explorer.CanEditData: explorer.EditDataCommand.Execute(null); - e.Handled = true; break; - case Models.DatabaseNodeType.View when explorer.CanBrowseData: + case DatabaseNodeType.View when explorer.CanBrowseData: explorer.SelectTop1000Command.Execute(null); - e.Handled = true; break; + + default: + return; } + + // The tap that follows this press will toggle the row. Remember what to put back. + m_rowToLeaveAsItWas = item; + m_asItWas = item.IsExpanded; + + e.Handled = true; + } + + /// + /// A double click on a table opens its data and does nothing else - in particular it does not + /// open the row, which the tree does on its own and which nothing here can prevent. + /// + /// + /// The chevron and the arrow keys are how a row is opened, and they are unaffected: this puts + /// back only the row a double click has just opened the data of. + /// + private void OnDoubleTappedAfterTheRow(object? sender, TappedEventArgs e) + { + if (m_rowToLeaveAsItWas is not { } row) + return; + + m_rowToLeaveAsItWas = null; + + row.IsExpanded = m_asItWas; } /// @@ -132,17 +183,9 @@ private void OnDoubleTapped(object? sender, Avalonia.Input.TappedEventArgs e) /// just opened. /// /// - private void OnPointerPressed(object? sender, PointerPressedEventArgs e) + private void OpenInTheBackground(PointerPressedEventArgs e) { - if (!e.GetCurrentPoint(this).Properties.IsMiddleButtonPressed) - return; - - var item = (e.Source as Visual)? - .GetSelfAndVisualAncestors() - .OfType() - .FirstOrDefault(); - - if (item?.DataContext is not DatabaseNode node) + if (NodeUnder(e) is not { } node) return; var explorer = ApplicationViewModel.Instance.DatabaseExplorerVm; @@ -156,6 +199,48 @@ private void OnPointerPressed(object? sender, PointerPressedEventArgs e) e.Handled = true; } + /// + /// The two clicks the TREE owns, both read from the pointer and both before the row sees them. + /// + private void OnPointerPressed(object? sender, PointerPressedEventArgs e) + { + var properties = e.GetCurrentPoint(this).Properties; + + if (properties.IsMiddleButtonPressed) + { + OpenInTheBackground(e); + return; + } + + if (properties.IsLeftButtonPressed && e.ClickCount == 2) + OpenTheDataUnderThePointer(e); + } + + /// The row the pointer is over, if it is over one. + private static TreeViewItem? ItemUnder(PointerPressedEventArgs e) + { + return (e.Source as Visual)? + .GetSelfAndVisualAncestors() + .OfType() + .FirstOrDefault(); + } + + /// The node whose row the pointer is over, if it is over one. + private static DatabaseNode? NodeUnder(PointerPressedEventArgs e) + { + return ItemUnder(e)?.DataContext as DatabaseNode; + } + + /// Whether the press landed on the row's expander rather than on the row. + private static bool PressedOnTheChevron(PointerPressedEventArgs e) + { + return (e.Source as Visual)? + .GetSelfAndVisualAncestors() + .TakeWhile(visual => visual is not TreeViewItem) + .OfType