From 3a9607fe0cbd9d607977197dec8f76ba4e7ab47a Mon Sep 17 00:00:00 2001 From: John Lambert Date: Wed, 19 Aug 2026 04:19:39 -0400 Subject: [PATCH] LT-22710: add failing tests for direct editing of rule formula cells Adds the failing tests that demonstrate Docs/bugs/phon-rule-direct-editing.md: - RuleFormulaVcBaseEditabilityTests: RuleFormulaVcBase.Display never sets ktptEditable=NotEditable before AddStringAltMember for the natural-class abbreviation (kfragNC) or the terminal-unit name (kfragTerminalUnit), across RegRuleFormulaVc, MetaRuleFormulaVc, and AffixRuleFormulaVc. - RuleFormulaDirectEditReproTests: drives a real IVwRootBox/PatternView and calls IVwSelection.ReplaceWithTsString directly (bypassing PatternView.OnKeyPress entirely, the same low-level path IME composition or drag-and-drop would use) and shows it actually renames the live PhPhoneme.Name. All 5 tests fail against current code, confirming the defect by direct reproduction rather than code reading alone. LT-22710: make rule formula fragments and rootsite non-editable Enforces "a rule cell is not free text" structurally instead of patching another input path: - RuleFormulaVcBase.Display now sets ktptEditable=NotEditable before every AddStringAltMember/AddProp call that binds a fragment to a real domain object's field (natural class abbreviation/name, terminal unit name) or to a computed feature/variable line. This is shared by RegRuleFormulaVc, MetaRuleFormulaVc, and AffixRuleFormulaVc, so all three rule kinds are covered by one change. - RuleFormulaControl now sets m_view.ReadOnlyView = true. This also unregisters the keyboard/IME controller hook for the view (see SimpleRootSite.ReadOnlyView), closing the IME-composition bypass, not just the WM_CHAR path PatternView.OnKeyPress already filtered. - PatternView.AllowDisplaySelection now always returns true (the established pattern also used by InterlinPrintView/InterlinTaggingChild), so the now-read-only rootsite still shows a visible selection for chooser insert/delete to act on. - PatternView.OnKeyPress is left in place as defence in depth. Adds a test confirming Delete still raises RemoveItemsRequested with the rootsite read-only, and disposes the test view to avoid a finalizer-thread COM cleanup race. All 6 reproduction/fix tests pass; MorphologyEditorDllTests (13), LexTextControlsTests (356), and ITextDllTests (208) show no regressions. LT-22710: add architecture self-review for the direct-editing fix Documents which layer owns the invariant, an experiment isolating that the view-constructor ktptEditable marking (not ReadOnlyView) is what actually blocks IVwSelection.ReplaceWithTsString, what was deliberately left unremoved (PatternEditingHelper's CanCut/CanPaste, shared with ComplexConcControl) and unfixed (ComplexConcPatternVc, unaudited fake-tag/literal spans elsewhere in the VC family), and manual verification still needed in a running FLEx. LT-22710: close mutation-testing coverage gaps - Add RuleFormulaControlWiringTests: constructs the real Reg/Meta/Affix rule formula controls and asserts ReadOnlyView on the shipped rootsite, so the production RuleFormulaControl.cs wiring is covered rather than only the test-only view built in RuleFormulaDirectEditReproTests. - Extend EditabilityRecordingEnv to record AddProp calls (previously it threw NotImplementedException on the exact call kfragFeature/kfragPlusVariable/ kfragMinusVariable make) and add one test per fragment. - Add AllowDisplaySelection_IsTrue_WhenRootsiteIsReadOnly. - Add the natural-class equivalent of the phoneme ReplaceWithTsString repro: builds a real PhSimpleContextNC special-cased to display only its abbreviation, selects it through the real rootbox, and confirms a direct ReplaceWithTsString cannot rename PhNaturalClass.Abbreviation -- the same end-to-end standard already applied to the phoneme path. Each addition was confirmed by ablation: flipping RuleFormulaControl's ReadOnlyView back to false, deleting the three feature/variable ktptEditable lines, removing AllowDisplaySelection, and removing kfragNC's ktptEditable line each turn exactly the new, targeted test(s) red and nothing else; ablating kfragNC's marking also reproduces "CORRUPTED" in PhNaturalClass.Abbreviation end-to-end, mirroring the phoneme case. Also corrects the review doc's characterization of ComplexConcPatternVc: it has no real domain-field bindings to corrupt, so a direct edit throws NotImplementedException out of UpdateProp rather than renaming anything -- a crash risk, not a data-corruption risk. Notes the audit surface is closed (PatternVcBase has exactly two subclasses, PatternView exactly two consumers) and flags ConstChartVc's apparent cell-level guard as unverified/SUSPECTED-safe rather than confirmed clean. LT-22710: move investigation and design notes to the PR description The bug analysis and architecture self-review were working documents for this fix. Their conclusions are now carried by the code and its tests; the reasoning, decisions and paths not taken live in the pull request body so they inform review without merging into the tree. LT-22710: tighten comments to the repo commenting standard State why a fragment is not editable rather than restating what the next line renders, drop a pointer to a test class and one to another view constructor's internals, and cut an over-long implementation comment to the sentence that matters. Re-wrap lines past the width limit. LT-22710: mark the formula table once instead of five fragments Each rule formula now marks its outermost table not editable and lets ktptEditable inherit down the box tree, replacing the five per-fragment markings. The three cases marked are the three root fragments the product actually renders: RegRuleFormulaVc.kfragRHS, MetaRuleFormulaVc.kfragRule and AffixRuleFormulaVc.kfragRule. One marking covers what five could not reach: the bracket glyphs, the fake tag spans and the zero-width-space boundaries. It also makes the cursor's landing place deliberate. Insert, delete and click all restore the cursor with fEditable true, so a cell has to keep one editable position; the boundary spans are now put back to editable on purpose rather than surviving because the shared bracket props happened to omit the property. Leaving them editable reaches no data, since ktagLeftBoundary and ktagRightBoundary are fake tags and UpdateProp returns the value unchanged. Stop overriding CanCopy. Cut and paste stay refused because they would change the rule, but a read-only view still has to let the formula be copied out. Set ReadOnlyView from the constructors rather than the designer region, where a designer round-trip could rewrite it, and record why it must run before MakeRoot: the setter forces MaxParasToScan to 0 once the rootbox exists, which would silently undo the arrow-key fix. Add STA to the fixture that drives a real IVwRootBox, since the Views COM objects are apartment-threaded and NUnit 3 defaults to MTA. Retarget the editability tests. The per-fragment assertions described a mechanism that no longer exists, so they now assert only that those fragments bind straight to a referenced object's live field. Three new cases pin the table marking, one pins the boundary opt-in, and one drives a live rootbox to require that an editable position still exists. Removing the opt-in fails the last two, which is how the cursor invariant is now held. --- Src/LexText/LexTextControls/PatternVcBase.cs | 21 + Src/LexText/LexTextControls/PatternView.cs | 19 +- Src/LexText/Morphology/AffixRuleFormulaVc.cs | 1 + Src/LexText/Morphology/MetaRuleFormulaVc.cs | 1 + .../RuleFormulaControlWiringTests.cs | 46 ++ .../RuleFormulaDirectEditReproTests.cs | 309 ++++++++++++ .../RuleFormulaVcBaseEditabilityTests.cs | 447 ++++++++++++++++++ Src/LexText/Morphology/RegRuleFormulaVc.cs | 1 + Src/LexText/Morphology/RuleFormulaControl.cs | 21 +- Src/LexText/Morphology/RuleFormulaVcBase.cs | 27 +- 10 files changed, 886 insertions(+), 7 deletions(-) create mode 100644 Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlWiringTests.cs create mode 100644 Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaDirectEditReproTests.cs create mode 100644 Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcBaseEditabilityTests.cs diff --git a/Src/LexText/LexTextControls/PatternVcBase.cs b/Src/LexText/LexTextControls/PatternVcBase.cs index ad4bd25fb7..575ef87b28 100644 --- a/Src/LexText/LexTextControls/PatternVcBase.cs +++ b/Src/LexText/LexTextControls/PatternVcBase.cs @@ -213,6 +213,25 @@ protected void AddExtraLines(int numLines, int tag, IVwEnv vwenv) } } + /// + /// Keeps the zero-width-space boundary spans editable even inside a pattern that has + /// been marked not editable as a whole. + /// + /// + /// Clicking an item, and every insert and delete, place the cursor through + /// MakeTextSelInObj with fEditable true, so a cell has to offer at least one editable + /// position or those commands stop working with no error. These boundary spans are the + /// position. They are safe to leave editable because ktagLeftBoundary and + /// ktagRightBoundary are fake tags rather than model properties, and + /// RuleFormulaVcBase.UpdateProp returns the value unchanged, so an edit landing here + /// reaches nothing real. + /// + protected static void MarkBoundaryEditable(IVwEnv vwenv) + { + vwenv.set_IntProperty((int)FwTextPropType.ktptEditable, (int)FwTextPropVar.ktpvEnum, + (int)TptEditable.ktptIsEditable); + } + protected void OpenSingleLinePile(IVwEnv vwenv, int maxNumLines) { OpenSingleLinePile(vwenv, maxNumLines, true); @@ -227,6 +246,7 @@ protected void OpenSingleLinePile(IVwEnv vwenv, int maxNumLines, bool addBoundar if (addBoundary) { vwenv.Props = m_bracketProps; + MarkBoundaryEditable(vwenv); vwenv.AddProp(ktagLeftBoundary, this, kfragZeroWidthSpace); } } @@ -241,6 +261,7 @@ protected void CloseSingleLinePile(IVwEnv vwenv, bool addBoundary) if (addBoundary) { vwenv.Props = m_bracketProps; + MarkBoundaryEditable(vwenv); vwenv.AddProp(ktagRightBoundary, this, kfragZeroWidthSpace); } vwenv.CloseParagraph(); diff --git a/Src/LexText/LexTextControls/PatternView.cs b/Src/LexText/LexTextControls/PatternView.cs index 27bbb76eca..5bc2e4162b 100644 --- a/Src/LexText/LexTextControls/PatternView.cs +++ b/Src/LexText/LexTextControls/PatternView.cs @@ -38,11 +38,11 @@ public PatternEditingHelper(LcmCache cache, IEditingCallbacks callbacks) { } - public override bool CanCopy() - { - return false; - } - + /// + /// Cut and paste stay refused because they would change the rule. Copy is not + /// overridden: a read-only view still has to let the formula be copied out, and the + /// base implementation already requires a real range selection. + /// public override bool CanCut() { return false; @@ -69,6 +69,15 @@ protected override EditingHelper CreateEditingHelper() return new PatternEditingHelper(Cache, this); } + /// + /// Activate() is disabled by default in ReadOnlyViews, but a pattern editor does want to + /// show selections so the user can see what a chooser insert/delete will act on. + /// + protected override bool AllowDisplaySelection + { + get { return true; } + } + public void Init(Mediator mediator, PropertyTable propertyTable, int hvo, IPatternControl patternControl, PatternVcBase vc, int rootFrag, ISilDataAccess sda) { CheckDisposed(); diff --git a/Src/LexText/Morphology/AffixRuleFormulaVc.cs b/Src/LexText/Morphology/AffixRuleFormulaVc.cs index e0e8b3333b..244b019e18 100644 --- a/Src/LexText/Morphology/AffixRuleFormulaVc.cs +++ b/Src/LexText/Morphology/AffixRuleFormulaVc.cs @@ -140,6 +140,7 @@ public override void Display(IVwEnv vwenv, int hvo, int frag) VwLength tableLen; tableLen.nVal = 10000; tableLen.unit = VwUnit.kunPercent100; + MarkFormulaNotEditable(vwenv); vwenv.OpenTable(3, tableLen, 0, VwAlignment.kvaLeft, VwFramePosition.kvfpVoid, VwRule.kvrlNone, 0, 0, false); VwLength inputLen; diff --git a/Src/LexText/Morphology/MetaRuleFormulaVc.cs b/Src/LexText/Morphology/MetaRuleFormulaVc.cs index 417a852a04..90999af101 100644 --- a/Src/LexText/Morphology/MetaRuleFormulaVc.cs +++ b/Src/LexText/Morphology/MetaRuleFormulaVc.cs @@ -103,6 +103,7 @@ public override void Display(IVwEnv vwenv, int hvo, int frag) VwLength tableLen; tableLen.nVal = 10000; tableLen.unit = VwUnit.kunPercent100; + MarkFormulaNotEditable(vwenv); vwenv.OpenTable(5, tableLen, 0, VwAlignment.kvaCenter, VwFramePosition.kvfpVoid, VwRule.kvrlNone, 0, 4000, false); VwLength ctxtLen; diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlWiringTests.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlWiringTests.cs new file mode 100644 index 0000000000..abfecba4a7 --- /dev/null +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlWiringTests.cs @@ -0,0 +1,46 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using NUnit.Framework; + +namespace SIL.FieldWorks.XWorks.MorphologyEditor +{ + /// + /// Constructing each real rule formula control must produce a read-only rootsite, since a + /// rule cell is modifiable only by chooser-insert and delete. + /// + [TestFixture] + public class RuleFormulaControlWiringTests + { + [Test] + public void RegRuleFormulaControl_RootSiteIsReadOnly() + { + using (var control = new RegRuleFormulaControl(null)) + { + Assert.That(control.RootSite.ReadOnlyView, Is.True, + "RegRuleFormulaControl must wire up a read-only rootsite"); + } + } + + [Test] + public void MetaRuleFormulaControl_RootSiteIsReadOnly() + { + using (var control = new MetaRuleFormulaControl(null)) + { + Assert.That(control.RootSite.ReadOnlyView, Is.True, + "MetaRuleFormulaControl must wire up a read-only rootsite"); + } + } + + [Test] + public void AffixRuleFormulaControl_RootSiteIsReadOnly() + { + using (var control = new AffixRuleFormulaControl(null)) + { + Assert.That(control.RootSite.ReadOnlyView, Is.True, + "AffixRuleFormulaControl must wire up a read-only rootsite"); + } + } + } +} diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaDirectEditReproTests.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaDirectEditReproTests.cs new file mode 100644 index 0000000000..9b6287a62d --- /dev/null +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaDirectEditReproTests.cs @@ -0,0 +1,309 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System.Windows.Forms; +using System.Threading; +using NUnit.Framework; +using SIL.LCModel; +using SIL.LCModel.Core.Text; +using SIL.LCModel.Core.KernelInterfaces; +using SIL.LCModel.Infrastructure; +using SIL.FieldWorks.Common.RootSites; +using SIL.FieldWorks.Common.ViewsInterfaces; +using SIL.FieldWorks.LexText.Controls; +using XCore; + +namespace SIL.FieldWorks.XWorks.MorphologyEditor +{ + /// + /// Drives a real IVwRootBox (the managed Views engine), hosted by a live + /// PatternView/RegRuleFormulaVc pair against a real in-memory LcmCache, and calls + /// IVwSelection.ReplaceWithTsString directly -- the same low-level entry point IME + /// composition or drag-and-drop would use, and one PatternView.OnKeyPress never sees + /// because it only reacts to Windows key events. + /// + [TestFixture] + // Drives a real IVwRootBox through MakeTextSelInObj and ReplaceWithTsString, and the + // Views COM objects are registered Apartment-threaded. NUnit 3 defaults to MTA. + [Apartment(ApartmentState.STA)] + public class RuleFormulaDirectEditReproTests : MemoryOnlyBackendProviderTestBase + { + private Mediator m_mediator; + private PropertyTable m_propertyTable; + private TestPatternView m_view; + + public override void TestSetup() + { + base.TestSetup(); + m_mediator = new Mediator(); + m_propertyTable = new PropertyTable(m_mediator); + m_propertyTable.SetProperty("cache", Cache, false); + } + + public override void TestTearDown() + { + if (m_view != null) + { + m_view.Dispose(); + m_view = null; + } + if (m_propertyTable != null) + { + m_propertyTable.Dispose(); + m_propertyTable = null; + } + if (m_mediator != null) + { + m_mediator.Dispose(); + m_mediator = null; + } + base.TestTearDown(); + } + + /// Minimal no-op IPatternControl -- sufficient because we never drive + /// selection through the chooser/insert/delete UI in this test; we only need + /// PatternView's selection-changed handler not to crash when we install a + /// selection directly. + private class NullPatternControl : IPatternControl + { + public object GetContext(SelectionHelper sel) => null; + public object GetContext(SelectionHelper sel, SelectionHelper.SelLimitType limit) => null; + public object GetItem(SelectionHelper sel, SelectionHelper.SelLimitType limit) => null; + public int GetItemContextIndex(object ctxt, object obj) => -1; + public SelLevInfo[] GetLevelInfo(object ctxt, int index) => null; + public int GetContextCount(object ctxt) => 0; + public object GetNextContext(object ctxt) => null; + public object GetPrevContext(object ctxt) => null; + public int GetFlid(object ctxt) => 0; + } + + /// Exposes the protected layout hook so the view can be laid out + /// headlessly. + private class TestPatternView : PatternView + { + public void CallLayout() + { + OnLayout(new LayoutEventArgs(this, string.Empty)); + } + + public void SimulateKeyDown(Keys key) + { + var e = new KeyEventArgs(key); + OnKeyDown(e); + } + + public bool TestAllowDisplaySelection => AllowDisplaySelection; + } + + private IPhPhoneme CreatePhoneme(string name) + { + IPhPhoneme p = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + Cache.LangProject.PhonologicalDataOA.PhonemeSetsOS.Add( + Cache.ServiceLocator.GetInstance().Create()); + p = Cache.ServiceLocator.GetInstance().Create(); + Cache.LangProject.PhonologicalDataOA.PhonemeSetsOS[0].PhonemesOC.Add(p); + p.Name.SetVernacularDefaultWritingSystem(name); + }); + return p; + } + + /// + /// Builds a real regular-rule RHS whose left context is a single phoneme, hosts it in a + /// live PatternView/RegRuleFormulaVc pair, and returns the phoneme plus the live view. + /// + private (IPhPhoneme phoneme, TestPatternView view) BuildLiveRuleFormulaView(string phonemeName) + { + IPhPhoneme phoneme = CreatePhoneme(phonemeName); + IPhSegRuleRHS rhs = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + var rule = Cache.ServiceLocator.GetInstance().Create(); + Cache.LangProject.PhonologicalDataOA.PhonRulesOS.Add(rule); + rhs = Cache.ServiceLocator.GetInstance().Create(); + rule.RightHandSidesOS.Add(rhs); + var segCtxt = Cache.ServiceLocator.GetInstance().Create(); + rhs.LeftContextOA = segCtxt; + segCtxt.FeatureStructureRA = phoneme; + }); + + var vc = new RegRuleFormulaVc(Cache, m_propertyTable); + var view = new TestPatternView { Cache = Cache, Visible = false, Width = 300, Height = 60 }; + view.Init(m_mediator, m_propertyTable, rhs.Hvo, new NullPatternControl(), vc, RegRuleFormulaVc.kfragRHS, + Cache.MainCacheAccessor); + view.ReadOnlyView = true; + view.CallLayout(); + m_view = view; + return (phoneme, view); + } + + /// + /// Builds a real regular-rule RHS whose left context is a natural class special-cased to + /// display only its abbreviation ("C" or "V"), + /// hosts it in a live PatternView/RegRuleFormulaVc pair, and returns the natural class + /// plus the live view. + /// + private (IPhNaturalClass naturalClass, TestPatternView view) BuildLiveRuleFormulaViewWithNaturalClass(string abbr) + { + IPhNaturalClass nc = null; + IPhSegRuleRHS rhs = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + nc = Cache.ServiceLocator.GetInstance().Create(); + Cache.LangProject.PhonologicalDataOA.NaturalClassesOS.Add(nc); + nc.Name.SetAnalysisDefaultWritingSystem("Test Class"); + nc.Abbreviation.SetAnalysisDefaultWritingSystem(abbr); + + var rule = Cache.ServiceLocator.GetInstance().Create(); + Cache.LangProject.PhonologicalDataOA.PhonRulesOS.Add(rule); + rhs = Cache.ServiceLocator.GetInstance().Create(); + rule.RightHandSidesOS.Add(rhs); + var ncCtxt = Cache.ServiceLocator.GetInstance().Create(); + rhs.LeftContextOA = ncCtxt; + ncCtxt.FeatureStructureRA = nc; + // GetNumLines(ncCtxt) must be exactly 1 to hit the "C"/"V" abbreviation-only + // branch; one plus-constraint variable is the cheapest way to make that so. + var constraint = Cache.ServiceLocator.GetInstance().Create(); + Cache.LangProject.PhonologicalDataOA.FeatConstraintsOS.Add(constraint); + ncCtxt.PlusConstrRS.Add(constraint); + }); + + var vc = new RegRuleFormulaVc(Cache, m_propertyTable); + var view = new TestPatternView { Cache = Cache, Visible = false, Width = 300, Height = 60 }; + view.Init(m_mediator, m_propertyTable, rhs.Hvo, new NullPatternControl(), vc, RegRuleFormulaVc.kfragRHS, + Cache.MainCacheAccessor); + view.ReadOnlyView = true; + view.CallLayout(); + m_view = view; + return (nc, view); + } + + /// + /// Selects the whole displayed natural-class abbreviation (via its object path from the + /// RHS root, bypassing PatternView.OnKeyPress entirely) and replaces its text directly + /// through IVwSelection.ReplaceWithTsString. A natural class is shared by every rule that + /// references it, so an edit landing here is a project-wide rename, exactly like the + /// phoneme case. + /// + [Test] + public void ReplaceWithTsString_OnNaturalClassAbbreviation_BypassesOnKeyPress_AndShouldNotRenameTheClass() + { + var (naturalClass, view) = BuildLiveRuleFormulaViewWithNaturalClass("C"); + + var levels = new[] + { + new SelLevInfo { tag = PhSimpleContextNCTags.kflidFeatureStructure, ihvo = 0 }, + new SelLevInfo { tag = PhSegRuleRHSTags.kflidLeftContext, ihvo = 0 } + }; + IVwSelection sel = view.RootBox.MakeTextSelInObj(0, levels.Length, levels, 0, null, + true, false, false, /* fWholeObj */ true, /* fInstall */ true); + Assert.That(sel, Is.Not.Null, + "could not construct a selection over the natural class's abbreviation display -- fixture/path assumption is wrong"); + + ITsString corrupted = TsStringUtils.MakeString("CORRUPTED", Cache.DefaultAnalWs); + + UndoableUnitOfWorkHelper.Do("undo", "redo", naturalClass, () => sel.ReplaceWithTsString(corrupted)); + + string abbrAfter = naturalClass.Abbreviation.AnalysisDefaultWritingSystem.Text; + Assert.That(abbrAfter, Is.EqualTo("C"), + "an edit that bypassed PatternView.OnKeyPress altered the real, project-wide " + + "PhNaturalClass.Abbreviation (got '" + abbrAfter + "')"); + } + + /// + /// Selects the whole displayed phoneme (via its object path from the RHS root, bypassing + /// any WM_CHAR-level filtering entirely -- PatternView.OnKeyPress is never invoked here) + /// and replaces its text directly through IVwSelection.ReplaceWithTsString, exactly the + /// kind of call an IME composition commit or a drag-and-drop would make. + /// + [Test] + public void ReplaceWithTsString_OnPhonemeTerminalUnit_BypassesOnKeyPress_AndShouldNotRenameThePhoneme() + { + var (phoneme, view) = BuildLiveRuleFormulaView("p"); + + var levels = new[] + { + new SelLevInfo { tag = PhSimpleContextSegTags.kflidFeatureStructure, ihvo = 0 }, + new SelLevInfo { tag = PhSegRuleRHSTags.kflidLeftContext, ihvo = 0 } + }; + IVwSelection sel = view.RootBox.MakeTextSelInObj(0, levels.Length, levels, 0, null, + true, false, false, /* fWholeObj */ true, /* fInstall */ true); + Assert.That(sel, Is.Not.Null, + "could not construct a selection over the phoneme's terminal-unit display -- fixture/path assumption is wrong"); + + ITsString corrupted = TsStringUtils.MakeString("CORRUPTED", Cache.DefaultVernWs); + + // The rootsite's own low-level text-replacement API, which bypasses the WM_CHAR + // filter. The unit of work is required for any edit to commit, not part of the + // bypass. + UndoableUnitOfWorkHelper.Do("undo", "redo", phoneme, () => sel.ReplaceWithTsString(corrupted)); + + string nameAfter = phoneme.Name.VernacularDefaultWritingSystem.Text; + Assert.That(nameAfter, Is.EqualTo("p"), + "an edit that bypassed PatternView.OnKeyPress altered the real, project-wide " + + "PhPhoneme.Name (got '" + nameAfter + "')"); + } + + /// + /// A read-only rootsite must not prevent PatternView's own Delete-key handling, which + /// removes items through RemoveItemsRequested rather than by editing text. + /// + [Test] + public void DeleteKey_StillRaisesRemoveItemsRequested_WhenRootsiteIsReadOnly() + { + var (_, view) = BuildLiveRuleFormulaView("p"); + Assert.That(view.ReadOnlyView, Is.True, "fixture assumption: the rootsite is read-only"); + + bool removeRequested = false; + view.RemoveItemsRequested += (sender, e) => removeRequested = true; + + view.SimulateKeyDown(Keys.Delete); + + Assert.That(removeRequested, Is.True, + "Delete must still raise RemoveItemsRequested when the rootsite is read-only"); + } + + /// + /// A formula marked not editable as a whole must still offer one editable position, or + /// clicking an item and every chooser insert and delete stop working with no exception + /// and nothing to see. The zero-width-space boundary spans are that position. + /// + /// + /// MakeSimpleSel returns a null selection rather than failing when it cannot find a + /// position with the requested characteristics, so a null return here is the regression + /// this guards: PatternView.GetSelectionInfo gives up on a non-editable selection, and + /// RuleFormulaControl.ReconstructView restores the cursor with fEditable true after + /// every insert and delete. + /// + [Test] + public void RuleFormula_StillOffersAnEditablePosition_WhenMarkedNotEditable() + { + var (phoneme, view) = BuildLiveRuleFormulaView("p"); + Assert.That(view.ReadOnlyView, Is.True, "fixture assumption: the rootsite is read-only"); + + IVwSelection editable = view.RootBox.MakeSimpleSel(true, true, false, true); + + Assert.That(editable, Is.Not.Null, + "the formula must keep an editable position for the cursor to land on"); + Assert.That(phoneme.Name.VernacularDefaultWritingSystem.Text, Is.EqualTo("p"), + "placing the cursor must not have altered the referenced phoneme"); + } + + /// + /// A read-only rootsite suppresses Activate() by default + /// (SimpleRootSite.AllowDisplaySelection), + /// which would hide the selection a chooser insert/delete needs the user to see. + /// + [Test] + public void AllowDisplaySelection_IsTrue_WhenRootsiteIsReadOnly() + { + var (_, view) = BuildLiveRuleFormulaView("p"); + Assert.That(view.ReadOnlyView, Is.True, "fixture assumption: the rootsite is read-only"); + + Assert.That(view.TestAllowDisplaySelection, Is.True, + "the selection must still be shown even though the rootsite is read-only"); + } + } +} diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcBaseEditabilityTests.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcBaseEditabilityTests.cs new file mode 100644 index 0000000000..458f0bfe97 --- /dev/null +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcBaseEditabilityTests.cs @@ -0,0 +1,447 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.InteropServices; +using NUnit.Framework; +using SIL.LCModel; +using SIL.LCModel.Core.KernelInterfaces; +using SIL.LCModel.Infrastructure; +using SIL.FieldWorks.Common.ViewsInterfaces; +using SIL.FieldWorks.LexText.Controls; +using XCore; + +namespace SIL.FieldWorks.XWorks.MorphologyEditor +{ + /// + /// The rule formula view is modifiable only by chooser-insert and delete, so each formula + /// marks its outermost table not editable and lets ktptEditable inherit down the box tree. + /// These tests pin that marking, pin the boundary spans being put back to editable so the + /// cursor still has a position, and record which fragments bind straight to a referenced + /// object's own live string field -- the write channel LT-22710 was about. + /// + [TestFixture] + public class RuleFormulaVcBaseEditabilityTests : MemoryOnlyBackendProviderTestBase + { + private Mediator m_mediator; + private PropertyTable m_propertyTable; + + public override void TestSetup() + { + base.TestSetup(); + m_mediator = new Mediator(); + m_propertyTable = new PropertyTable(m_mediator); + } + + public override void TestTearDown() + { + if (m_propertyTable != null) + { + m_propertyTable.Dispose(); + m_propertyTable = null; + } + if (m_mediator != null) + { + m_mediator.Dispose(); + m_mediator = null; + } + base.TestTearDown(); + } + + private IPhNaturalClass CreateNaturalClass(string abbr) + { + IPhNaturalClass nc = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + nc = Cache.ServiceLocator.GetInstance().Create(); + Cache.LangProject.PhonologicalDataOA.NaturalClassesOS.Add(nc); + nc.Name.SetAnalysisDefaultWritingSystem("Test Class"); + nc.Abbreviation.SetAnalysisDefaultWritingSystem(abbr); + }); + return nc; + } + + private IPhPhoneme CreatePhoneme(string name) + { + IPhPhoneme p = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + Cache.LangProject.PhonologicalDataOA.PhonemeSetsOS.Add( + Cache.ServiceLocator.GetInstance().Create()); + p = Cache.ServiceLocator.GetInstance().Create(); + Cache.LangProject.PhonologicalDataOA.PhonemeSetsOS[0].PhonemesOC.Add(p); + p.Name.SetVernacularDefaultWritingSystem(name); + }); + return p; + } + + /// + /// The natural-class abbreviation fragment (kfragNC) is rendered via AddStringAltMember + /// directly against the natural class's own Abbreviation field. + /// + [Test] + public void Display_NaturalClassAbbreviationFragment_BindsDirectlyToTheReferencedField() + { + IPhNaturalClass nc = CreateNaturalClass("Stp"); + var vc = new RegRuleFormulaVc(Cache, m_propertyTable); + var env = new EditabilityRecordingEnv(); + + vc.Display(env, nc.Hvo, RuleFormulaVcBase.kfragNC); + + Assert.That(env.StringAltMemberCalls, Is.Not.Empty, + "expected AddStringAltMember to be called for the NC abbreviation fragment"); + } + + /// + /// The terminal-unit (phoneme/boundary) fragment (kfragTerminalUnit) is rendered via + /// AddStringAltMember directly against the terminal unit's own Name field -- a live + /// write channel into the phoneme's real, project-wide name. + /// + [Test] + public void Display_TerminalUnitNameFragment_BindsDirectlyToTheReferencedField() + { + IPhPhoneme phoneme = CreatePhoneme("p"); + var vc = new RegRuleFormulaVc(Cache, m_propertyTable); + var env = new EditabilityRecordingEnv(); + + vc.Display(env, phoneme.Hvo, RuleFormulaVcBase.kfragTerminalUnit); + + Assert.That(env.StringAltMemberCalls, Is.Not.Empty, + "expected AddStringAltMember to be called for the terminal unit name fragment"); + } + + /// Same defect, exercised through the metathesis-rule view + /// constructor. + [Test] + public void Display_TerminalUnitNameFragment_ViaMetaRuleFormulaVc_BindsDirectlyToTheReferencedField() + { + IPhPhoneme phoneme = CreatePhoneme("t"); + var vc = new MetaRuleFormulaVc(Cache, m_propertyTable); + var env = new EditabilityRecordingEnv(); + + vc.Display(env, phoneme.Hvo, RuleFormulaVcBase.kfragTerminalUnit); + + Assert.That(env.StringAltMemberCalls, Is.Not.Empty); + } + + /// Same defect, exercised through the affix-process view constructor. + [Test] + public void Display_TerminalUnitNameFragment_ViaAffixRuleFormulaVc_BindsDirectlyToTheReferencedField() + { + IPhPhoneme phoneme = CreatePhoneme("k"); + var vc = new AffixRuleFormulaVc(Cache, m_propertyTable); + var env = new EditabilityRecordingEnv(); + + vc.Display(env, phoneme.Hvo, RuleFormulaVcBase.kfragTerminalUnit); + + Assert.That(env.StringAltMemberCalls, Is.Not.Empty); + } + + /// + /// The feature-value line (kfragFeature) is a computed "abbreviation value" string bound + /// to a fake tag, not free text. + /// + [Test] + public void Display_FeatureLineFragment_BindsDirectlyToTheReferencedField() + { + var vc = new RegRuleFormulaVc(Cache, m_propertyTable); + var env = new EditabilityRecordingEnv(); + + vc.Display(env, 0, RuleFormulaVcBase.kfragFeature); + + Assert.That(env.AddPropCalls, Is.Not.Empty, + "expected AddProp to be called for the feature-line fragment"); + } + + /// The plus-variable line (kfragPlusVariable) is a computed string bound to a + /// fake tag, not free text. + [Test] + public void Display_PlusVariableLineFragment_BindsDirectlyToTheReferencedField() + { + var vc = new RegRuleFormulaVc(Cache, m_propertyTable); + var env = new EditabilityRecordingEnv(); + + vc.Display(env, 0, RuleFormulaVcBase.kfragPlusVariable); + + Assert.That(env.AddPropCalls, Is.Not.Empty, + "expected AddProp to be called for the plus-variable fragment"); + } + + /// The minus-variable line (kfragMinusVariable) is a computed string bound to a + /// fake tag, not free text. + [Test] + public void Display_MinusVariableLineFragment_BindsDirectlyToTheReferencedField() + { + var vc = new RegRuleFormulaVc(Cache, m_propertyTable); + var env = new EditabilityRecordingEnv(); + + vc.Display(env, 0, RuleFormulaVcBase.kfragMinusVariable); + + Assert.That(env.AddPropCalls, Is.Not.Empty, + "expected AddProp to be called for the minus-variable fragment"); + } + + /// + /// Every rule formula must mark itself not editable on its outermost table, because + /// ktptEditable inherits down the box tree and that is what covers the referenced + /// phoneme and natural-class fields, the computed lines, the bracket glyphs and the + /// fake-tag spans in one place. + /// + /// + /// Display is driven only as far as the first OpenTable; the fixture's IVwEnv throws on + /// the member reached next, which is the point of the expected exception. + /// + [TestCase("Reg")] + [TestCase("Meta")] + [TestCase("Affix")] + public void Display_RuleFormulaRootFragment_MarksTheWholeTableNotEditable(string ruleKind) + { + var env = new EditabilityRecordingEnv(); + int rootFrag; + RuleFormulaVcBase vc = BuildVcForRootFragment(ruleKind, out rootFrag, out int rootHvo); + + Assert.Throws(() => vc.Display(env, rootHvo, rootFrag), + "expected Display to run past the outermost OpenTable and then hit an " + + "unimplemented IVwEnv member"); + + Assert.That(env.OpenTableCalls, Is.Not.Empty, + "expected the formula to open its outermost table"); + Assert.That(env.OpenTableCalls[0].EditableAtCallTime, + Is.EqualTo((int)TptEditable.ktptNotEditable), + "the outermost table must already be marked not editable, so that every string " + + "inside the formula inherits it"); + } + + /// + /// The zero-width-space boundary spans must be put back to editable inside a formula + /// that is not editable as a whole, because clicking an item and every insert and + /// delete place the cursor with fEditable true and need somewhere to land. + /// + [Test] + public void OpenAndCloseSingleLinePile_MarkTheBoundarySpansEditable() + { + var env = new EditabilityRecordingEnv(); + var vc = new BoundaryProbeVc(Cache, m_propertyTable); + + vc.ProbeBoundaries(env); + + var boundaryCalls = env.AddPropCalls + .Where(call => call.Tag == PatternVcBase.ktagLeftBoundary + || call.Tag == PatternVcBase.ktagRightBoundary) + .ToList(); + Assert.That(boundaryCalls, Has.Count.EqualTo(2), + "expected both the left and the right boundary span"); + foreach (var call in boundaryCalls) + { + Assert.That(call.EditableAtCallTime, Is.EqualTo((int)TptEditable.ktptIsEditable), + "the boundary span must stay editable so the cursor has a position in the " + + "cell; ktagLeftBoundary and ktagRightBoundary are fake tags, so an edit " + + "landing here reaches no model data"); + } + } + + private RuleFormulaVcBase BuildVcForRootFragment(string ruleKind, out int rootFrag, + out int rootHvo) + { + IPhSegRuleRHS rhs = null; + IMoAffixProcess affixRule = null; + IPhMetathesisRule metathesisRule = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + switch (ruleKind) + { + case "Reg": + var regRule = Cache.ServiceLocator.GetInstance().Create(); + Cache.LangProject.PhonologicalDataOA.PhonRulesOS.Add(regRule); + rhs = Cache.ServiceLocator.GetInstance().Create(); + regRule.RightHandSidesOS.Add(rhs); + break; + case "Meta": + metathesisRule = Cache.ServiceLocator.GetInstance().Create(); + Cache.LangProject.PhonologicalDataOA.PhonRulesOS.Add(metathesisRule); + break; + default: + // A MoAffixProcess has to be owned as a lexical entry's form. + var entry = Cache.ServiceLocator.GetInstance().Create(); + affixRule = Cache.ServiceLocator.GetInstance().Create(); + entry.LexemeFormOA = affixRule; + break; + } + }); + + switch (ruleKind) + { + case "Reg": + rootFrag = RegRuleFormulaVc.kfragRHS; + rootHvo = rhs.Hvo; + return new RegRuleFormulaVc(Cache, m_propertyTable); + case "Meta": + rootFrag = MetaRuleFormulaVc.kfragRule; + rootHvo = metathesisRule.Hvo; + return new MetaRuleFormulaVc(Cache, m_propertyTable); + default: + rootFrag = AffixRuleFormulaVc.kfragRule; + rootHvo = affixRule.Hvo; + return new AffixRuleFormulaVc(Cache, m_propertyTable); + } + } + + /// Reaches the protected pile helpers so the boundary spans can be observed + /// without building a whole rule. + private class BoundaryProbeVc : RegRuleFormulaVc + { + public BoundaryProbeVc(LcmCache cache, PropertyTable propertyTable) + : base(cache, propertyTable) + { + } + + public void ProbeBoundaries(IVwEnv vwenv) + { + OpenSingleLinePile(vwenv, 1); + CloseSingleLinePile(vwenv); + } + } + + /// + /// Records enough of IVwEnv's calls to observe which tag a fragment binds to, and what + /// the ktptEditable property was most recently set to when a table is opened or a + /// boundary span is added. All other members are unused by the fragments under test and + /// throw if hit, so a future change that routes through a different IVwEnv member will + /// fail loudly rather than silently pass. + /// + private class EditabilityRecordingEnv : IVwEnv + { + public struct Call + { + public int Tag; + public int Ws; + public int EditableAtCallTime; + } + + public struct PropCall + { + public int Tag; + public int Frag; + public int EditableAtCallTime; + } + + public struct OpenTableCall + { + public int EditableAtCallTime; + } + + public List StringAltMemberCalls = new List(); + public List AddPropCalls = new List(); + public List OpenTableCalls = new List(); + + private int m_currentEditable = int.MinValue; // sentinel: never set + + public void AddStringAltMember(int tag, int ws, IVwViewConstructor _vwvc) + { + StringAltMemberCalls.Add(new Call { Tag = tag, Ws = ws, EditableAtCallTime = m_currentEditable }); + } + + public void AddProp(int tag, IVwViewConstructor _vwvc, int frag) + { + AddPropCalls.Add(new PropCall { Tag = tag, Frag = frag, EditableAtCallTime = m_currentEditable }); + } + + public void set_IntProperty(int tpt, int tpv, int nValue) + { + if (tpt == (int)FwTextPropType.ktptEditable) + m_currentEditable = nValue; + } + + public ITsTextProps Props + { + set { /* the recorded editable state comes from set_IntProperty, not from whole props */ } + } + + public void get_StringWidth(ITsString _tss, ITsTextProps _ttp, out int dmpx, out int dmpy) + { + dmpx = 0; + dmpy = 0; + } + + public int OpenObject + { + get { throw new NotImplementedException(); } + } + + public int EmbeddingLevel + { + get { return 0; } + } + + public ISilDataAccess DataAccess + { + get { throw new NotImplementedException(); } + } + + public void AddObjProp(int tag, IVwViewConstructor _vwvc, int frag) { throw new NotImplementedException(); } + public void AddObjVec(int tag, IVwViewConstructor _vwvc, int frag) { throw new NotImplementedException(); } + public void AddObjVecItems(int tag, IVwViewConstructor _vwvc, int frag) { throw new NotImplementedException(); } + public void AddReversedObjVecItems(int tag, IVwViewConstructor _vwvc, int frag) { throw new NotImplementedException(); } + public void AddObj(int hvo, IVwViewConstructor _vwvc, int frag) { throw new NotImplementedException(); } + public void AddLazyVecItems(int tag, IVwViewConstructor _vwvc, int frag) { throw new NotImplementedException(); } + public void AddLazyItems(int[] _rghvo, int chvo, IVwViewConstructor _vwvc, int frag) { throw new NotImplementedException(); } + public void AddDerivedProp(int[] _rgtag, int ctag, IVwViewConstructor _vwvc, int frag) { throw new NotImplementedException(); } + public void NoteDependency(int[] _rghvo, int[] _rgtag, int chvo) { } + public void NoteStringValDependency(int hvo, int tag, int ws, ITsString _tssVal) { throw new NotImplementedException(); } + public void AddStringProp(int tag, IVwViewConstructor _vwvc) { throw new NotImplementedException(); } + public void AddUnicodeProp(int tag, int ws, IVwViewConstructor _vwvc) { throw new NotImplementedException(); } + public void AddIntProp(int tag) { throw new NotImplementedException(); } + public void AddIntPropPic(int tag, IVwViewConstructor _vc, int frag, int nMin, int nMax) { throw new NotImplementedException(); } + public void AddStringAlt(int tag) { throw new NotImplementedException(); } + public void AddStringAltSeq(int tag, int[] _rgenc, int cws) { throw new NotImplementedException(); } + public void AddString(ITsString _ss) { throw new NotImplementedException(); } + public void AddTimeProp(int tag, uint flags) { throw new NotImplementedException(); } + public int CurrentObject() { throw new NotImplementedException(); } + public void GetOuterObject(int ichvoLevel, out int _hvo, out int _tag, out int _ihvo) { throw new NotImplementedException(); } + public void AddWindow(IVwEmbeddedWindow _ew, int dmpAscent, bool fJustifyRight, bool fAutoShow) { throw new NotImplementedException(); } + public void AddSeparatorBar() { throw new NotImplementedException(); } + public void AddSimpleRect(int rgb, int dmpWidth, int dmpHeight, int dmpBaselineOffset) { throw new NotImplementedException(); } + public void OpenDiv() { throw new NotImplementedException(); } + public void CloseDiv() { throw new NotImplementedException(); } + public void OpenParagraph() { } + public void OpenTaggedPara() { throw new NotImplementedException(); } + public void OpenMappedPara() { throw new NotImplementedException(); } + public void OpenMappedTaggedPara() { throw new NotImplementedException(); } + public void OpenConcPara(int ichMinItem, int ichLimItem, VwConcParaOpts cpoFlags, int dmpAlign) { throw new NotImplementedException(); } + public void OpenOverridePara(int cOverrideProperties, DispPropOverride[] _rgOverrideProperties) { throw new NotImplementedException(); } + public void CloseParagraph() { } + public void OpenInnerPile() { } + public void CloseInnerPile() { } + public void OpenSpan() { throw new NotImplementedException(); } + public void CloseSpan() { throw new NotImplementedException(); } + public void OpenTable(int cCols, VwLength vlWidth, int mpBorder, VwAlignment vwalign, VwFramePosition frmpos, VwRule vwrule, int mpSpacing, int mpPadding, bool fSelectOneCol) + { + OpenTableCalls.Add(new OpenTableCall { EditableAtCallTime = m_currentEditable }); + } + public void CloseTable() { throw new NotImplementedException(); } + public void OpenTableRow() { throw new NotImplementedException(); } + public void CloseTableRow() { throw new NotImplementedException(); } + public void OpenTableCell(int nRowSpan, int nColSpan) { throw new NotImplementedException(); } + public void CloseTableCell() { throw new NotImplementedException(); } + public void OpenTableHeaderCell(int nRowSpan, int nColSpan) { throw new NotImplementedException(); } + public void CloseTableHeaderCell() { throw new NotImplementedException(); } + public void MakeColumns(int nColSpan, VwLength vlWidth) { throw new NotImplementedException(); } + public void MakeColumnGroup(int nColSpan, VwLength vlWidth) { throw new NotImplementedException(); } + public void OpenTableHeader() { throw new NotImplementedException(); } + public void CloseTableHeader() { throw new NotImplementedException(); } + public void OpenTableFooter() { throw new NotImplementedException(); } + public void CloseTableFooter() { throw new NotImplementedException(); } + public void OpenTableBody() { throw new NotImplementedException(); } + public void CloseTableBody() { throw new NotImplementedException(); } + public void set_StringProperty(int sp, string bstrValue) { } + public void AddPictureWithCaption(IPicture _pict, int tag, ITsTextProps _ttpCaption, int hvoCmFile, int ws, int dxmpWidth, int dympHeight, IVwViewConstructor _vwvc) { throw new NotImplementedException(); } + public void AddPicture(IPicture _pict, int tag, int dxmpWidth, int dympHeight) { throw new NotImplementedException(); } + public void SetParagraphMark(VwBoundaryMark boundaryMark) { throw new NotImplementedException(); } + public void EmptyParagraphBehavior(int behavior) { throw new NotImplementedException(); } + public bool IsParagraphOpen() { throw new NotImplementedException(); } + } + } +} diff --git a/Src/LexText/Morphology/RegRuleFormulaVc.cs b/Src/LexText/Morphology/RegRuleFormulaVc.cs index 58aeacf4d8..cd7d36a172 100644 --- a/Src/LexText/Morphology/RegRuleFormulaVc.cs +++ b/Src/LexText/Morphology/RegRuleFormulaVc.cs @@ -111,6 +111,7 @@ public override void Display(IVwEnv vwenv, int hvo, int frag) VwLength tableLen; tableLen.nVal = 10000; tableLen.unit = VwUnit.kunPercent100; + MarkFormulaNotEditable(vwenv); vwenv.OpenTable(7, tableLen, 0, VwAlignment.kvaCenter, VwFramePosition.kvfpVoid, VwRule.kvrlNone, 0, 0, false); VwLength ctxtLen; diff --git a/Src/LexText/Morphology/RuleFormulaControl.cs b/Src/LexText/Morphology/RuleFormulaControl.cs index 88035e5afa..ea1df93156 100644 --- a/Src/LexText/Morphology/RuleFormulaControl.cs +++ b/Src/LexText/Morphology/RuleFormulaControl.cs @@ -112,12 +112,32 @@ public override string ToString() public RuleFormulaControl() { InitializeComponent(); + MakeViewReadOnly(); } public RuleFormulaControl(XmlNode configurationNode) { m_configurationNode = configurationNode; InitializeComponent(); + MakeViewReadOnly(); + } + + /// + /// Makes the formula view read-only. A cell changes only through chooser-insert and + /// delete, never through free text; see LT-22710. + /// + /// + /// This must run before PatternView.MakeRoot, and it does, because both constructors + /// call it while m_rootb is still null. SimpleRootSite's ReadOnlyView setter forces + /// MaxParasToScan to 0 whenever the rootbox already exists, and MakeRoot deliberately + /// raises it to 10 so the arrow keys can cross the non-editable empty lines in a pile. + /// Setting ReadOnlyView on a live PatternView would undo that silently, so keep this + /// out of MakeRoot and out of the designer region, which a designer round-trip can + /// rewrite. + /// + private void MakeViewReadOnly() + { + m_view.ReadOnlyView = true; } public RootSite RootSite @@ -1150,7 +1170,6 @@ private void InitializeComponent() this.m_view.Location = new System.Drawing.Point(0, 0); this.m_view.Mediator = null; this.m_view.Name = "m_view"; - this.m_view.ReadOnlyView = false; this.m_view.ScrollMinSize = new System.Drawing.Size(0, 0); this.m_view.ScrollPosition = new System.Drawing.Point(0, 0); this.m_view.ShowRangeSelAfterLostFocus = false; diff --git a/Src/LexText/Morphology/RuleFormulaVcBase.cs b/Src/LexText/Morphology/RuleFormulaVcBase.cs index b316717761..d981c4523c 100644 --- a/Src/LexText/Morphology/RuleFormulaVcBase.cs +++ b/Src/LexText/Morphology/RuleFormulaVcBase.cs @@ -59,6 +59,29 @@ protected RuleFormulaVcBase(LcmCache cache, PropertyTable propertyTable) m_x = TsStringUtils.MakeString("X", userWs); } + /// + /// Marks everything a rule formula draws as not editable. Call immediately before the + /// formula's outermost OpenTable, so that ktptEditable inherits down the whole box tree. + /// + /// + /// Every string in a formula is either a referenced object's own field -- a phoneme's + /// Name, a natural class's Abbreviation -- or a value computed for display. Typing into + /// any of them renames the referenced object for every rule that uses it, which is + /// LT-22710. Nothing here is free text: PatternView.OnKeyPress discards every character + /// except Backspace and Delete (LT-21888). + /// + /// Marking the table rather than each fragment also covers the bracket glyphs, the fake + /// tag spans and the zero-width-space boundaries, none of which a per-fragment marking + /// reaches. CloseSingleLinePile and OpenSingleLinePile put ktptIsEditable back on just + /// the boundary spans, which is where the cursor has to be able to land; see + /// PatternVcBase. + /// + protected static void MarkFormulaNotEditable(IVwEnv vwenv) + { + vwenv.set_IntProperty((int)FwTextPropType.ktptEditable, (int)FwTextPropVar.ktpvEnum, + (int)TptEditable.ktptNotEditable); + } + /// /// Gets the maximum number of lines for context cells. /// @@ -335,7 +358,9 @@ public override void Display(IVwEnv vwenv, int hvo, int frag) public override ITsString DisplayVariant(IVwEnv vwenv, int tag, int frag) { - // we use display variant to display literal strings that are editable + // we use display variant to display literal strings that are not backed by a model + // property; the formula marks the whole table not editable, so none of them accept + // typing ITsString tss; switch (frag) {