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) {