From 4ed6f7be3a01a926c8c1ba0fdb4cf8326844daef Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Mon, 19 Oct 2026 21:50:40 -0400 Subject: [PATCH 1/4] LT-22690: Test ghost add rows, and create nothing when one is cleared Typing into a ghost row creates its object on the first keystroke, so typing and then clearing the row before leaving it saved an empty object. The edit context now records which row opened the edit session. Clearing a ghost row whose edits are the whole session cancels that session: nothing is created and no undo step remains. An edit another row staged into the same session is never cancelled. New tests cover the ticket's acceptance steps. Typing into the Example and SubSenses ghost rows creates the object and the view re-composes into its rows. Creating and typing are one undo step, and undoing it brings the ghost row back. Focusing and leaving an untouched ghost row stages and saves nothing. Co-Authored-By: Claude Opus 5.5 --- .../FwAvaloniaTests/DetailEditingTests.cs | 6 +- .../Composer/ComposedDetailEditContext.cs | 10 +- Src/xWorks/Avalonia/DetailEditContextBase.cs | 29 ++++ .../Composer/DetailEditContextEditingTests.cs | 154 ++++++++++++++++++ 4 files changed, 197 insertions(+), 2 deletions(-) diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditingTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditingTests.cs index 227779c159..db2eb9cfdb 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditingTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditingTests.cs @@ -1127,7 +1127,8 @@ public void GhostRow_WatermarkClearsOnFocus_AndRestoresWhenLeftEmpty() isEditable: true, indent: 0, ghostPrompt: "Click here to add Lexeme Form"); var model = new DetailModel("LexEntry", "Normal", new List { ghost }, new List()); - var view = new DataTree(model, new FakeDetailEditContext()); + var context = new FakeDetailEditContext(); + var view = new DataTree(model, context); var window = new Window { Content = view, Width = 480, Height = 200 }; window.Show(); Dispatcher.UIThread.RunJobs(); @@ -1144,6 +1145,9 @@ public void GhostRow_WatermarkClearsOnFocus_AndRestoresWhenLeftEmpty() Dispatcher.UIThread.RunJobs(); Assert.That(box.Watermark, Is.EqualTo("Click here to add Lexeme Form"), "leaving without typing restores the prompt"); + Assert.That(context.TextEdits, Is.Empty, + "focusing and leaving the ghost row stages nothing, so nothing is created"); + Assert.That(context.CommitCount, Is.Zero, "with nothing staged there is nothing to save"); } // A search-backed reference vector (SearchOptions diff --git a/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs b/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs index 809100f393..7ca4fac91c 100644 --- a/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs +++ b/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs @@ -82,8 +82,16 @@ public override bool TrySetText(DetailField field, string ws, string value) var setter = Handler(field)?.Text; if (setter == null) return false; + // A ghost row cleared before the user leaves it creates nothing (LT-22690): when its + // edits are the whole open session, roll back the object its first keystroke made. + if (string.IsNullOrEmpty(value) && field.GhostPrompt != null + && IsSessionOwnedBy(field.StableId)) + { + Cancel(); + return true; + } // A single field's edit names the undo label (e.g. "Undo change to Gloss"). - return Stage(() => setter(ws, value), FieldLabelFor(field)); + return Stage(() => setter(ws, value), FieldLabelFor(field), field.StableId); } public override bool TrySetRichText(DetailField field, string ws, DetailRichTextValue value) diff --git a/Src/xWorks/Avalonia/DetailEditContextBase.cs b/Src/xWorks/Avalonia/DetailEditContextBase.cs index 721d201110..24a94c59ac 100644 --- a/Src/xWorks/Avalonia/DetailEditContextBase.cs +++ b/Src/xWorks/Avalonia/DetailEditContextBase.cs @@ -21,6 +21,9 @@ namespace SIL.FieldWorks.XWorks public abstract class DetailEditContextBase : IDetailEditContext { private LcmDetailEditSession _session; + // The owner key of every write in the open session, or null once writes under + // different keys (or no key) have mixed in. + private string _sessionOwnerKey; private readonly Dictionary> _pendingEditFlushes = new Dictionary>(StringComparer.Ordinal); @@ -154,15 +157,27 @@ public bool FlushPendingEdits() /// public void Commit() { + _sessionOwnerKey = null; _session?.Commit(); } /// public void Cancel() { + _sessionOwnerKey = null; _session?.Cancel(); } + /// + /// Whether the open session holds only edits staged under : + /// a call with that key opened it, and + /// every later staged write used the same key. False when no session is open or the key + /// is null. + /// + protected bool IsSessionOwnedBy(string ownerKey) + => IsOpen && ownerKey != null + && string.Equals(_sessionOwnerKey, ownerKey, StringComparison.Ordinal); + /// /// Opens the fenced session on the first staged edit; later calls are no-ops. The generic /// "Undo/Redo Edit Entry" label is used -- the batch/bulk path that stages several fields @@ -230,10 +245,24 @@ public bool Stage(Func setter) /// already-open session leaves that session's label untouched. /// public bool Stage(Func setter, string fieldLabel) + { + return Stage(setter, fieldLabel, null); + } + + /// + /// As , and records the write under + /// for . A null key, or a key + /// different from the one that opened the session, leaves the session owned by no one. + /// + protected bool Stage(Func setter, string fieldLabel, string ownerKey) { if (setter == null) return false; var wasOpen = IsOpen; + if (!wasOpen) + _sessionOwnerKey = ownerKey; + else if (!string.Equals(_sessionOwnerKey, ownerKey, StringComparison.Ordinal)) + _sessionOwnerKey = null; EnsureOpen(fieldLabel); try { diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs index c87ba390d5..a3f0981d8f 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs @@ -1555,6 +1555,160 @@ public void Compose_GhostWithoutGhostField_IsNotEditable_SoTypingCannotCreateAnd "no bare object was created behind the user's back"); } + // The sense's Examples ghost row (ghost="Example" ghostWs="vernacular"). + private static DetailField ExampleGhost(ComposedDetail composed, ILexSense sense) + => composed.Model.Fields.FirstOrDefault(f => f.Field == "Examples" + && f.StableId.EndsWith(DetailField.GhostStableIdSuffix) && f.ObjectHvo == sense.Hvo); + + // The sense's own Senses ghost row, labelled SubSenses. The layout gives it no ghost= + // attribute; LT-22690 approves the row, typing into it the new subsense's Gloss. + private static DetailField SubsenseGhost(ComposedDetail composed, ILexSense sense) + => composed.Model.Fields.FirstOrDefault(f => f.Field == "Senses" + && f.StableId.EndsWith(DetailField.GhostStableIdSuffix) && f.ObjectHvo == sense.Hvo); + + // Stages text the way the view does, one value per keystroke. + private static void TypeInto(ComposedDetail composed, DetailField field, string text) + { + for (var length = 1; length <= text.Length; length++) + { + Assert.That(composed.EditContext.TrySetText(field, field.Values[0].WsAbbrev, + text.Substring(0, length)), Is.True, "each keystroke stages"); + } + } + + [Test] + public void Compose_GhostExample_TypingCreatesTheExample_AndRecomposesIntoItsRows() + { + var sense = m_entry.SensesOS[0]; + var composed = DetailComposer.Compose(m_entry, Cache); + var ghost = ExampleGhost(composed, sense); + Assert.That(ghost, Is.Not.Null, "a sense with no examples shows the Example ghost row"); + Assert.That(ghost.IsEditable, Is.True); + Assert.That(ghost.GhostPrompt, Does.Contain("Example")); + + TypeInto(composed, ghost, "una casa"); + composed.EditContext.Commit(); + + Assert.That(sense.ExamplesOS.Count, Is.EqualTo(1), "typing created the example"); + var example = sense.ExamplesOS[0]; + Assert.That(example.Example.get_String(Cache.DefaultVernWs).Text, Is.EqualTo("una casa"), + "the typed text landed in the Example field, in the vernacular writing system"); + + var recomposed = DetailComposer.Compose(m_entry, Cache); + Assert.That(ExampleGhost(recomposed, sense), Is.Null, + "with an example present the ghost row gives way to the example's own rows"); + var exampleRow = recomposed.Model.Fields.FirstOrDefault(f => + f.Field == "Example" && f.ObjectHvo == example.Hvo); + Assert.That(exampleRow, Is.Not.Null, "the new example composes its Example row"); + Assert.That(exampleRow.Values.Any(v => v.Value == "una casa"), Is.True, + "the Example row shows the typed text"); + } + + [Test] + public void Compose_GhostExample_CreationAndTyping_AreOneUndoStep() + { + var sense = m_entry.SensesOS[0]; + var composed = DetailComposer.Compose(m_entry, Cache); + var stepsBefore = Cache.ActionHandlerAccessor.UndoableSequenceCount; + + TypeInto(composed, ExampleGhost(composed, sense), "una casa"); + composed.EditContext.Commit(); + Assert.That(Cache.ActionHandlerAccessor.UndoableSequenceCount, Is.EqualTo(stepsBefore + 1), + "creating the example and every keystroke typed into it are one undo step"); + + Cache.ActionHandlerAccessor.Undo(); + Assert.That(sense.ExamplesOS.Count, Is.Zero, "one undo removes the example"); + Assert.That(ExampleGhost(DetailComposer.Compose(m_entry, Cache), sense), Is.Not.Null, + "after the undo the sense shows its Example ghost row again"); + } + + [Test] + public void Compose_GhostExample_TypedThenCleared_CreatesNothing() + { + var sense = m_entry.SensesOS[0]; + var composed = DetailComposer.Compose(m_entry, Cache); + var ghost = ExampleGhost(composed, sense); + var stepsBefore = Cache.ActionHandlerAccessor.UndoableSequenceCount; + + TypeInto(composed, ghost, "un"); + Assert.That(sense.ExamplesOS.Count, Is.EqualTo(1), "the first keystroke creates the example"); + Assert.That(composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, "u"), Is.True); + Assert.That(composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, ""), Is.True, + "clearing the row is accepted"); + var sessionOpenAfterClear = composed.EditContext.IsOpen; + var examplesAfterClear = sense.ExamplesOS.Count; + // Closes the session before asserting, so a failure here cannot leave the next + // test's setup inside an open undo task. + composed.EditContext.Commit(); + + Assert.That(sessionOpenAfterClear, Is.False, "clearing the row ends its edit session"); + Assert.That(examplesAfterClear, Is.Zero, "clearing the row removes the example it created"); + Assert.That(Cache.ActionHandlerAccessor.UndoableSequenceCount, Is.EqualTo(stepsBefore), + "nothing reaches the undo stack, not even a step that changes nothing"); + + TypeInto(composed, ghost, "b"); + composed.EditContext.Commit(); + Assert.That(sense.ExamplesOS.Count, Is.EqualTo(1), "typing again creates the example afresh"); + Assert.That(sense.ExamplesOS[0].Example.get_String(Cache.DefaultVernWs).Text, Is.EqualTo("b")); + } + + // Clearing the ghost row must not cancel an edit another row staged into the same + // session. + [Test] + public void Compose_GhostExample_ClearedAfterAnotherRowsEdit_KeepsThatEdit() + { + var sense = m_entry.SensesOS[0]; + var composed = DetailComposer.Compose(m_entry, Cache); + var gloss = composed.Model.Fields.First(f => f.Field == "Gloss" && f.ObjectHvo == sense.Hvo); + var ghost = ExampleGhost(composed, sense); + + var glossStaged = composed.EditContext.TrySetText(gloss, gloss.Values[0].WsAbbrev, "home"); + var typedStaged = composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, "u"); + var clearStaged = composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, ""); + var sessionOpenAfterClear = composed.EditContext.IsOpen; + // Closes the session before asserting, so a failure here cannot leave the next + // test's setup inside an open undo task. + composed.EditContext.Commit(); + + Assert.That(glossStaged && typedStaged && clearStaged, Is.True, "every edit stages"); + Assert.That(sessionOpenAfterClear, Is.True, + "the session holds another row's edit, so clearing the ghost row does not cancel it"); + Assert.That(sense.Gloss.get_String(Cache.DefaultAnalWs).Text, Is.EqualTo("home"), + "the other row's edit is saved"); + } + + [Test] + public void Compose_GhostSubsense_TypingCreatesASubsense_WithTheTypedGloss() + { + var sense = m_entry.SensesOS[0]; + var composed = DetailComposer.Compose(m_entry, Cache); + var ghost = SubsenseGhost(composed, sense); + Assert.That(ghost, Is.Not.Null, "a sense with no subsenses shows the SubSenses ghost row"); + Assert.That(ghost.IsEditable, Is.True); + var stepsBefore = Cache.ActionHandlerAccessor.UndoableSequenceCount; + + TypeInto(composed, ghost, "small house"); + composed.EditContext.Commit(); + + Assert.That(sense.SensesOS.Count, Is.EqualTo(1), "typing created the subsense"); + var subsense = sense.SensesOS[0]; + Assert.That(subsense.Gloss.get_String(Cache.DefaultAnalWs).Text, Is.EqualTo("small house"), + "the typed text became the subsense's gloss"); + Assert.That(Cache.ActionHandlerAccessor.UndoableSequenceCount, Is.EqualTo(stepsBefore + 1), + "creating the subsense and typing its gloss are one undo step"); + + var recomposed = DetailComposer.Compose(m_entry, Cache); + Assert.That(SubsenseGhost(recomposed, sense), Is.Null, + "with a subsense present the ghost row gives way to the subsense's own rows"); + Assert.That(recomposed.Model.Fields.Any(f => f.Field == "Gloss" && f.ObjectHvo == subsense.Hvo), + Is.True, "the new subsense composes its Gloss row"); + + Cache.ActionHandlerAccessor.Undo(); + Assert.That(sense.SensesOS.Count, Is.Zero, "one undo removes the subsense"); + Assert.That(SubsenseGhost(DetailComposer.Compose(m_entry, Cache), sense), Is.Not.Null, + "after the undo the sense shows its SubSenses ghost row again"); + } + // Conditional display: the real shipped variant/complex-form // divergence. LexEntryRef/Normal's VariantEntryTypes and ComplexEntryTypes parts are // twins (LexEntryParts.xml:1133-1162); exactly one may From ea981176dad58af7bfeb4c4939d790f9a3b46cad Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Tue, 20 Oct 2026 13:10:35 -0400 Subject: [PATCH 2/4] LT-22690: Trim the comments on the ghost row change Shorten the session-ownership doc comments to their contracts, and drop test comments that restate the helper or test name. Comment-only. Co-Authored-By: Claude Opus 5.5 --- .../Avalonia/Composer/ComposedDetailEditContext.cs | 3 +-- Src/xWorks/Avalonia/DetailEditContextBase.cs | 13 ++++--------- .../Composer/DetailEditContextEditingTests.cs | 14 ++++---------- 3 files changed, 9 insertions(+), 21 deletions(-) diff --git a/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs b/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs index 7ca4fac91c..b2ee999e2d 100644 --- a/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs +++ b/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs @@ -82,8 +82,7 @@ public override bool TrySetText(DetailField field, string ws, string value) var setter = Handler(field)?.Text; if (setter == null) return false; - // A ghost row cleared before the user leaves it creates nothing (LT-22690): when its - // edits are the whole open session, roll back the object its first keystroke made. + // A ghost row cleared while it owns the session creates nothing (LT-22690). if (string.IsNullOrEmpty(value) && field.GhostPrompt != null && IsSessionOwnedBy(field.StableId)) { diff --git a/Src/xWorks/Avalonia/DetailEditContextBase.cs b/Src/xWorks/Avalonia/DetailEditContextBase.cs index 24a94c59ac..0b6bf74014 100644 --- a/Src/xWorks/Avalonia/DetailEditContextBase.cs +++ b/Src/xWorks/Avalonia/DetailEditContextBase.cs @@ -21,8 +21,6 @@ namespace SIL.FieldWorks.XWorks public abstract class DetailEditContextBase : IDetailEditContext { private LcmDetailEditSession _session; - // The owner key of every write in the open session, or null once writes under - // different keys (or no key) have mixed in. private string _sessionOwnerKey; private readonly Dictionary> _pendingEditFlushes = new Dictionary>(StringComparer.Ordinal); @@ -169,10 +167,8 @@ public void Cancel() } /// - /// Whether the open session holds only edits staged under : - /// a call with that key opened it, and - /// every later staged write used the same key. False when no session is open or the key - /// is null. + /// Whether every write in the open session was staged under . + /// False when no session is open or the key is null. /// protected bool IsSessionOwnedBy(string ownerKey) => IsOpen && ownerKey != null @@ -250,9 +246,8 @@ public bool Stage(Func setter, string fieldLabel) } /// - /// As , and records the write under - /// for . A null key, or a key - /// different from the one that opened the session, leaves the session owned by no one. + /// As , recording the write under + /// for . /// protected bool Stage(Func setter, string fieldLabel, string ownerKey) { diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs index a3f0981d8f..144a582f28 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs @@ -1555,18 +1555,16 @@ public void Compose_GhostWithoutGhostField_IsNotEditable_SoTypingCannotCreateAnd "no bare object was created behind the user's back"); } - // The sense's Examples ghost row (ghost="Example" ghostWs="vernacular"). private static DetailField ExampleGhost(ComposedDetail composed, ILexSense sense) => composed.Model.Fields.FirstOrDefault(f => f.Field == "Examples" && f.StableId.EndsWith(DetailField.GhostStableIdSuffix) && f.ObjectHvo == sense.Hvo); - // The sense's own Senses ghost row, labelled SubSenses. The layout gives it no ghost= - // attribute; LT-22690 approves the row, typing into it the new subsense's Gloss. + // The layout gives SubSenses no ghost= attribute; LT-22690 approves its ghost row. private static DetailField SubsenseGhost(ComposedDetail composed, ILexSense sense) => composed.Model.Fields.FirstOrDefault(f => f.Field == "Senses" && f.StableId.EndsWith(DetailField.GhostStableIdSuffix) && f.ObjectHvo == sense.Hvo); - // Stages text the way the view does, one value per keystroke. + // One stage per keystroke, as the view does. private static void TypeInto(ComposedDetail composed, DetailField field, string text) { for (var length = 1; length <= text.Length; length++) @@ -1637,8 +1635,7 @@ public void Compose_GhostExample_TypedThenCleared_CreatesNothing() "clearing the row is accepted"); var sessionOpenAfterClear = composed.EditContext.IsOpen; var examplesAfterClear = sense.ExamplesOS.Count; - // Closes the session before asserting, so a failure here cannot leave the next - // test's setup inside an open undo task. + // Close the session first, so a failure cannot break the next test's setup. composed.EditContext.Commit(); Assert.That(sessionOpenAfterClear, Is.False, "clearing the row ends its edit session"); @@ -1652,8 +1649,6 @@ public void Compose_GhostExample_TypedThenCleared_CreatesNothing() Assert.That(sense.ExamplesOS[0].Example.get_String(Cache.DefaultVernWs).Text, Is.EqualTo("b")); } - // Clearing the ghost row must not cancel an edit another row staged into the same - // session. [Test] public void Compose_GhostExample_ClearedAfterAnotherRowsEdit_KeepsThatEdit() { @@ -1666,8 +1661,7 @@ public void Compose_GhostExample_ClearedAfterAnotherRowsEdit_KeepsThatEdit() var typedStaged = composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, "u"); var clearStaged = composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, ""); var sessionOpenAfterClear = composed.EditContext.IsOpen; - // Closes the session before asserting, so a failure here cannot leave the next - // test's setup inside an open undo task. + // Close the session first, so a failure cannot break the next test's setup. composed.EditContext.Commit(); Assert.That(glossStaged && typedStaged && clearStaged, Is.True, "every edit stages"); From 09f92df5ccdecc91c1c805d79b7b321c4d85dcdf Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Tue, 20 Oct 2026 13:26:44 -0400 Subject: [PATCH 3/4] Comment cleanup --- Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs b/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs index b2ee999e2d..4a03c3e5c1 100644 --- a/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs +++ b/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs @@ -7,7 +7,6 @@ using System.Linq; using SIL.FieldWorks.Common.FwAvalonia.Detail; using SIL.LCModel; -using SIL.LCModel.Core.KernelInterfaces; namespace SIL.FieldWorks.XWorks { @@ -82,7 +81,7 @@ public override bool TrySetText(DetailField field, string ws, string value) var setter = Handler(field)?.Text; if (setter == null) return false; - // A ghost row cleared while it owns the session creates nothing (LT-22690). + if (string.IsNullOrEmpty(value) && field.GhostPrompt != null && IsSessionOwnedBy(field.StableId)) { From 4c87d5a5e6a30079aa23208bf46026b99d0955b8 Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Tue, 20 Oct 2026 14:26:54 -0400 Subject: [PATCH 4/4] LT-22690: Cancel a cleared ghost row only when it can roll back Two gaps in clearing a ghost row before leaving it: - When the edit session joined an undo task opened elsewhere, cancelling it rolled nothing back, so the created Example kept the text the user had just cleared. The cancel path now also requires a session that can roll back; otherwise the empty text is written. - A rejected write from another row dropped the ghost row's ownership of the session. Ownership now changes only on a successful write, and is cleared when a setter throws in an already-open session. A test for each reproduces the gap and passes with its fix. Co-Authored-By: Claude Opus 5.5 --- .../Composer/ComposedDetailEditContext.cs | 2 +- .../Avalonia/Composer/LcmDetailEditSession.cs | 6 +++ Src/xWorks/Avalonia/DetailEditContextBase.cs | 25 +++++++--- .../Composer/DetailEditContextEditingTests.cs | 49 +++++++++++++++++++ 4 files changed, 75 insertions(+), 7 deletions(-) diff --git a/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs b/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs index 4a03c3e5c1..6e3e020906 100644 --- a/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs +++ b/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs @@ -83,7 +83,7 @@ public override bool TrySetText(DetailField field, string ws, string value) return false; if (string.IsNullOrEmpty(value) && field.GhostPrompt != null - && IsSessionOwnedBy(field.StableId)) + && IsSessionOwnedBy(field.StableId) && CanRollBackSession) { Cancel(); return true; diff --git a/Src/xWorks/Avalonia/Composer/LcmDetailEditSession.cs b/Src/xWorks/Avalonia/Composer/LcmDetailEditSession.cs index e2b51b175b..84f7a3ba07 100644 --- a/Src/xWorks/Avalonia/Composer/LcmDetailEditSession.cs +++ b/Src/xWorks/Avalonia/Composer/LcmDetailEditSession.cs @@ -43,6 +43,12 @@ public LcmDetailEditSession(LcmCache cache, string undoLabel, string redoLabel) /// public bool IsOpen { get; private set; } + /// + /// Whether would roll back this session's writes: it is open and + /// owns its undo task, which is still the open one. + /// + public bool CanRollBack => IsOpen && _ownsTask && TaskStillOpen; + /// public void Commit() { diff --git a/Src/xWorks/Avalonia/DetailEditContextBase.cs b/Src/xWorks/Avalonia/DetailEditContextBase.cs index 0b6bf74014..8b3802eed9 100644 --- a/Src/xWorks/Avalonia/DetailEditContextBase.cs +++ b/Src/xWorks/Avalonia/DetailEditContextBase.cs @@ -174,6 +174,12 @@ protected bool IsSessionOwnedBy(string ownerKey) => IsOpen && ownerKey != null && string.Equals(_sessionOwnerKey, ownerKey, StringComparison.Ordinal); + /// + /// Whether would roll back the open session's writes. False when the + /// session joined an undo task opened elsewhere, which only that task's opener can end. + /// + protected bool CanRollBackSession => _session != null && _session.CanRollBack; + /// /// Opens the fenced session on the first staged edit; later calls are no-ops. The generic /// "Undo/Redo Edit Entry" label is used -- the batch/bulk path that stages several fields @@ -246,7 +252,7 @@ public bool Stage(Func setter, string fieldLabel) } /// - /// As , recording the write under + /// As , recording a write that stages under /// for . /// protected bool Stage(Func setter, string fieldLabel, string ownerKey) @@ -254,22 +260,29 @@ protected bool Stage(Func setter, string fieldLabel, string ownerKey) if (setter == null) return false; var wasOpen = IsOpen; - if (!wasOpen) - _sessionOwnerKey = ownerKey; - else if (!string.Equals(_sessionOwnerKey, ownerKey, StringComparison.Ordinal)) - _sessionOwnerKey = null; EnsureOpen(fieldLabel); try { var staged = setter(); - if (!staged && !wasOpen) + if (staged) + { + if (!wasOpen) + _sessionOwnerKey = ownerKey; + else if (!string.Equals(_sessionOwnerKey, ownerKey, StringComparison.Ordinal)) + _sessionOwnerKey = null; + } + else if (!wasOpen) + { Cancel(); + } return staged; } catch { if (!wasOpen) Cancel(); + else + _sessionOwnerKey = null; // a throwing setter may have written before it threw throw; } } diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs index 144a582f28..09e914f471 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs @@ -1671,6 +1671,55 @@ public void Compose_GhostExample_ClearedAfterAnotherRowsEdit_KeepsThatEdit() "the other row's edit is saved"); } + [Test] + public void Compose_GhostExample_ClearedAfterAnotherRowsRejectedEdit_CreatesNothing() + { + var sense = m_entry.SensesOS[0]; + var composed = DetailComposer.Compose(m_entry, Cache); + var gloss = composed.Model.Fields.First(f => f.Field == "Gloss" && f.ObjectHvo == sense.Hvo); + var ghost = ExampleGhost(composed, sense); + + TypeInto(composed, ghost, "u"); + var glossStaged = composed.EditContext.TrySetText(gloss, "no-such-ws", "home"); + var clearStaged = composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, ""); + var sessionOpenAfterClear = composed.EditContext.IsOpen; + var examplesAfterClear = sense.ExamplesOS.Count; + // Close the session first, so a failure cannot break the next test's setup. + composed.EditContext.Commit(); + + Assert.That(glossStaged, Is.False, "precondition: the gloss row rejects an unknown writing system"); + Assert.That(clearStaged, Is.True); + Assert.That(sessionOpenAfterClear, Is.False, + "a rejected edit wrote nothing, so the session still holds only the ghost row's edits"); + Assert.That(examplesAfterClear, Is.Zero, "clearing the row removes the example it created"); + } + + // An undo task opened before the row is typed into belongs to its opener, and cancelling + // the edit session cannot roll it back. + [Test] + public void Compose_GhostExample_ClearedInsideAnOuterUndoTask_KeepsNoTypedText() + { + var sense = m_entry.SensesOS[0]; + var composed = DetailComposer.Compose(m_entry, Cache); + var ghost = ExampleGhost(composed, sense); + + Cache.DomainDataByFlid.BeginUndoTask("outer", "outer"); + try + { + TypeInto(composed, ghost, "u"); + Assert.That(composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, ""), Is.True, + "clearing the row is accepted"); + composed.EditContext.Commit(); + } + finally + { + Cache.DomainDataByFlid.EndUndoTask(); + } + + Assert.That(sense.ExamplesOS.Select(e => e.Example.get_String(Cache.DefaultVernWs).Text ?? ""), + Has.All.Empty, "the text the user cleared is not saved by the outer task"); + } + [Test] public void Compose_GhostSubsense_TypingCreatesASubsense_WithTheTypedGloss() {