diff --git a/Src/LexText/Interlinear/ComplexConcControl.Designer.cs b/Src/LexText/Interlinear/ComplexConcControl.Designer.cs index fd16c9b0d1..a03e049698 100644 --- a/Src/LexText/Interlinear/ComplexConcControl.Designer.cs +++ b/Src/LexText/Interlinear/ComplexConcControl.Designer.cs @@ -54,7 +54,10 @@ private void InitializeComponent() this.m_view.IsTextBox = false; this.m_view.Mediator = null; this.m_view.Name = "m_view"; - this.m_view.ReadOnlyView = false; + // The pattern builder is modifiable only via chooser-insert and delete, never free + // text: content is entirely computed from the synthetic pattern-node tree, so a + // typed or IME-composed edit has nothing valid to apply. + this.m_view.ReadOnlyView = true; 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/Interlinear/ComplexConcPatternVc.cs b/Src/LexText/Interlinear/ComplexConcPatternVc.cs index 7880bda032..c308dfe8aa 100644 --- a/Src/LexText/Interlinear/ComplexConcPatternVc.cs +++ b/Src/LexText/Interlinear/ComplexConcPatternVc.cs @@ -79,6 +79,7 @@ public override void Display(IVwEnv vwenv, int hvo, int frag) { OpenSingleLinePile(vwenv, GetMaxNumLines(vwenv), false); vwenv.Props = m_bracketProps; + SetNotEditable(vwenv); vwenv.AddProp(ComplexConcPatternSda.ktagChildren, this, kfragEmpty); CloseSingleLinePile(vwenv, false); } @@ -100,12 +101,14 @@ public override void Display(IVwEnv vwenv, int hvo, int frag) if (node is ComplexConcOrNode) { OpenSingleLinePile(vwenv, maxNumLines); + SetNotEditable(vwenv); vwenv.AddProp(ktagInnerNonBoundary, this, kfragOR); CloseSingleLinePile(vwenv, false); } else if (node is ComplexConcWordBdryNode) { OpenSingleLinePile(vwenv, maxNumLines); + SetNotEditable(vwenv); vwenv.AddProp(ktagInnerNonBoundary, this, kfragHash); CloseSingleLinePile(vwenv); } @@ -117,10 +120,12 @@ public override void Display(IVwEnv vwenv, int hvo, int frag) { OpenSingleLinePile(vwenv, maxNumLines, false); // use normal parentheses for a single line group + SetNotEditable(vwenv); vwenv.AddProp(ktagLeftBoundary, this, kfragLeftParen); vwenv.AddObjVecItems(ComplexConcPatternSda.ktagChildren, this, kfragNode); + SetNotEditable(vwenv); vwenv.AddProp(hasMinMax ? ktagInnerNonBoundary : ktagRightBoundary, this, kfragRightParen); if (hasMinMax) DisplayMinMax(numLines, vwenv); @@ -130,11 +135,17 @@ public override void Display(IVwEnv vwenv, int hvo, int frag) { vwenv.Props = m_bracketProps; vwenv.set_IntProperty((int) FwTextPropType.ktptMarginLeading, (int) FwTextPropVar.ktpvMilliPoint, PileMargin); + SetNotEditable(vwenv); vwenv.OpenInnerPile(); AddExtraLines(maxNumLines - numLines, ktagLeftNonBoundary, vwenv); + SetNotEditable(vwenv); vwenv.AddProp(ktagLeftNonBoundary, this, kfragLeftParenUpHook); for (int i = 1; i < numLines - 1; i++) + { + SetNotEditable(vwenv); vwenv.AddProp(ktagLeftNonBoundary, this, kfragLeftParenExt); + } + SetNotEditable(vwenv); vwenv.AddProp(ktagLeftBoundary, this, kfragLeftParenLowHook); vwenv.CloseInnerPile(); @@ -142,11 +153,17 @@ public override void Display(IVwEnv vwenv, int hvo, int frag) vwenv.Props = m_bracketProps; vwenv.set_IntProperty((int) FwTextPropType.ktptMarginTrailing, (int) FwTextPropVar.ktpvMilliPoint, PileMargin); + SetNotEditable(vwenv); vwenv.OpenInnerPile(); AddExtraLines(maxNumLines - numLines, hasMinMax ? ktagInnerNonBoundary : ktagRightNonBoundary, vwenv); + SetNotEditable(vwenv); vwenv.AddProp(hasMinMax ? ktagInnerNonBoundary : ktagRightNonBoundary, this, kfragRightParenUpHook); for (int i = 1; i < numLines - 1; i++) + { + SetNotEditable(vwenv); vwenv.AddProp(hasMinMax ? ktagInnerNonBoundary : ktagRightNonBoundary, this, kfragRightParenExt); + } + SetNotEditable(vwenv); vwenv.AddProp(hasMinMax ? ktagInnerNonBoundary : ktagRightBoundary, this, kfragRightParenLowHook); vwenv.CloseInnerPile(); if (hasMinMax) @@ -161,10 +178,12 @@ public override void Display(IVwEnv vwenv, int hvo, int frag) { OpenSingleLinePile(vwenv, maxNumLines, false); // use normal brackets for a single line constraint + SetNotEditable(vwenv); vwenv.AddProp(ktagLeftBoundary, this, kfragLeftBracket); DisplayFeatures(vwenv, node); + SetNotEditable(vwenv); vwenv.AddProp(hasMinMax ? ktagInnerNonBoundary : ktagRightBoundary, this, kfragRightBracket); if (hasMinMax) DisplayMinMax(numLines, vwenv); @@ -175,11 +194,17 @@ public override void Display(IVwEnv vwenv, int hvo, int frag) // left bracket pile vwenv.Props = m_bracketProps; vwenv.set_IntProperty((int) FwTextPropType.ktptMarginLeading, (int) FwTextPropVar.ktpvMilliPoint, PileMargin); + SetNotEditable(vwenv); vwenv.OpenInnerPile(); AddExtraLines(maxNumLines - numLines, ktagLeftNonBoundary, vwenv); + SetNotEditable(vwenv); vwenv.AddProp(ktagLeftNonBoundary, this, kfragLeftBracketUpHook); for (int i = 1; i < numLines - 1; i++) + { + SetNotEditable(vwenv); vwenv.AddProp(ktagLeftNonBoundary, this, kfragLeftBracketExt); + } + SetNotEditable(vwenv); vwenv.AddProp(ktagLeftBoundary, this, kfragLeftBracketLowHook); vwenv.CloseInnerPile(); @@ -193,11 +218,17 @@ public override void Display(IVwEnv vwenv, int hvo, int frag) // right bracket pile vwenv.Props = m_bracketProps; vwenv.set_IntProperty((int) FwTextPropType.ktptMarginTrailing, (int) FwTextPropVar.ktpvMilliPoint, PileMargin); + SetNotEditable(vwenv); vwenv.OpenInnerPile(); AddExtraLines(maxNumLines - numLines, hasMinMax ? ktagInnerNonBoundary : ktagRightNonBoundary, vwenv); + SetNotEditable(vwenv); vwenv.AddProp(hasMinMax ? ktagInnerNonBoundary : ktagRightNonBoundary, this, kfragRightBracketUpHook); for (int i = 1; i < numLines - 1; i++) + { + SetNotEditable(vwenv); vwenv.AddProp(hasMinMax ? ktagInnerNonBoundary : ktagRightNonBoundary, this, kfragRightBracketExt); + } + SetNotEditable(vwenv); vwenv.AddProp(hasMinMax ? ktagInnerNonBoundary : ktagRightBoundary, this, kfragRightBracketLowHook); vwenv.CloseInnerPile(); if (hasMinMax) @@ -208,6 +239,17 @@ public override void Display(IVwEnv vwenv, int hvo, int frag) } } + /// + /// Every fragment ComplexConcPatternVc renders -- feature lines, quantifiers, OR/word- + /// boundary literals, and the bracket/paren glyphs -- is a computed display, never free + /// text (see UpdateProp). Mark the current run explicitly non-editable so an edit attempt + /// is rejected at the selection layer instead of silently reaching UpdateProp. + /// + private static void SetNotEditable(IVwEnv vwenv) + { + vwenv.set_IntProperty((int) FwTextPropType.ktptEditable, (int) FwTextPropVar.ktpvEnum, (int) TptEditable.ktptNotEditable); + } + private void DisplayMinMax(int numLines, IVwEnv vwenv) { int superOffset = 0; @@ -230,11 +272,13 @@ private void DisplayMinMax(int numLines, IVwEnv vwenv) if (numLines == 1) vwenv.set_IntProperty((int) FwTextPropType.ktptOffset, (int) FwTextPropVar.ktpvMilliPoint, superOffset); vwenv.OpenParagraph(); + SetNotEditable(vwenv); vwenv.AddProp(ktagRightNonBoundary, this, kfragNodeMax); vwenv.CloseParagraph(); AddExtraLines(numLines - 2, ktagRightNonBoundary, vwenv); vwenv.set_IntProperty((int) FwTextPropType.ktptOffset, (int) FwTextPropVar.ktpvMilliPoint, 0); vwenv.OpenParagraph(); + SetNotEditable(vwenv); vwenv.AddProp(ktagRightBoundary, this, kfragNodeMin); vwenv.CloseParagraph(); vwenv.CloseInnerPile(); @@ -382,21 +426,38 @@ public override ITsString DisplayVariant(IVwEnv vwenv, int tag, int frag) private void DisplayFeatures(IVwEnv vwenv, ComplexConcPatternNode node) { + // Every line here (Type, Form, Entry, Category, Gloss, Infl Features) is a computed + // summary of the synthetic pattern node, not free text; see UpdateProp and + // SetNotEditable. + SetNotEditable(vwenv); vwenv.AddProp(ktagType, this, kfragFeatureLine); var morphNode = node as ComplexConcMorphNode; if (morphNode != null) { if (morphNode.Form != null) + { + SetNotEditable(vwenv); vwenv.AddProp(ktagForm, this, kfragFeatureLine); + } if (morphNode.Entry != null) + { + SetNotEditable(vwenv); vwenv.AddProp(ktagEntry, this, kfragFeatureLine); + } if (morphNode.Category != null) + { + SetNotEditable(vwenv); vwenv.AddProp(ktagCategory, this, kfragFeatureLine); + } if (morphNode.Gloss != null) + { + SetNotEditable(vwenv); vwenv.AddProp(ktagGloss, this, kfragFeatureLine); + } if (morphNode.InflFeatures.Count > 0) { vwenv.OpenParagraph(); + SetNotEditable(vwenv); vwenv.AddProp(ktagInfl, this, kfragFeatureLine); DisplayInflFeatures(vwenv, morphNode.InflFeatures); vwenv.CloseParagraph(); @@ -408,14 +469,24 @@ private void DisplayFeatures(IVwEnv vwenv, ComplexConcPatternNode node) if (wordNode != null) { if (wordNode.Form != null) + { + SetNotEditable(vwenv); vwenv.AddProp(ktagForm, this, kfragFeatureLine); + } if (wordNode.Category != null) + { + SetNotEditable(vwenv); vwenv.AddProp(ktagCategory, this, kfragFeatureLine); + } if (wordNode.Gloss != null) + { + SetNotEditable(vwenv); vwenv.AddProp(ktagGloss, this, kfragFeatureLine); + } if (wordNode.InflFeatures.Count > 0) { vwenv.OpenParagraph(); + SetNotEditable(vwenv); vwenv.AddProp(ktagInfl, this, kfragFeatureLine); DisplayInflFeatures(vwenv, wordNode.InflFeatures); vwenv.CloseParagraph(); @@ -427,7 +498,10 @@ private void DisplayFeatures(IVwEnv vwenv, ComplexConcPatternNode node) if (tagNode != null) { if (tagNode.Tag != null) + { + SetNotEditable(vwenv); vwenv.AddProp(ktagTag, this, kfragFeatureLine); + } } } } @@ -443,6 +517,7 @@ private void DisplayInflFeatureLines(IVwEnv vwenv, IDictionary) kvp.Value); if (openPara) @@ -450,6 +525,7 @@ private void DisplayInflFeatureLines(IVwEnv vwenv, IDictionary if (numLines == 1) { // use normal brackets for a single line constraint + SetNotEditable(vwenv); vwenv.AddProp(ktagInnerNonBoundary, this, kfragLeftBracket); DisplayInflFeatureLines(vwenv, inflFeatures, false); + SetNotEditable(vwenv); vwenv.AddProp(ktagInnerNonBoundary, this, kfragRightBracket); } else @@ -473,10 +551,16 @@ private void DisplayInflFeatures(IVwEnv vwenv, IDictionary // left bracket pile vwenv.Props = m_bracketProps; vwenv.set_IntProperty((int) FwTextPropType.ktptMarginLeading, (int) FwTextPropVar.ktpvMilliPoint, PileMargin); + SetNotEditable(vwenv); vwenv.OpenInnerPile(); + SetNotEditable(vwenv); vwenv.AddProp(ktagLeftNonBoundary, this, kfragLeftBracketUpHook); for (int i = 1; i < numLines - 1; i++) + { + SetNotEditable(vwenv); vwenv.AddProp(ktagLeftNonBoundary, this, kfragLeftBracketExt); + } + SetNotEditable(vwenv); vwenv.AddProp(ktagLeftBoundary, this, kfragLeftBracketLowHook); vwenv.CloseInnerPile(); @@ -489,15 +573,32 @@ private void DisplayInflFeatures(IVwEnv vwenv, IDictionary // right bracket pile vwenv.Props = m_bracketProps; vwenv.set_IntProperty((int) FwTextPropType.ktptMarginTrailing, (int) FwTextPropVar.ktpvMilliPoint, PileMargin); + SetNotEditable(vwenv); vwenv.OpenInnerPile(); + SetNotEditable(vwenv); vwenv.AddProp(ktagInnerNonBoundary, this, kfragRightBracketUpHook); for (int i = 1; i < numLines - 1; i++) + { + SetNotEditable(vwenv); vwenv.AddProp(ktagInnerNonBoundary, this, kfragRightBracketExt); + } + SetNotEditable(vwenv); vwenv.AddProp(ktagInnerNonBoundary, this, kfragRightBracketLowHook); vwenv.CloseInnerPile(); } } + /// + /// Every fragment this VC renders is a computed display (Display/DisplayVariant read + /// live node state), not a real bound field, so an edit that reaches this far has + /// nothing to apply. Absorb it and let the next layout redraw the correct value, the + /// same way RuleFormulaVcBase.UpdateProp does for the sibling rule-formula editor. + /// + public override ITsString UpdateProp(IVwSelection vwsel, int hvo, int tag, int frag, ITsString tssVal) + { + return tssVal; + } + public ITsString CreateFeatureLine(ITsString name, ITsString value, bool negated) { ITsIncStrBldr featLine = TsStringUtils.MakeIncStrBldr(); diff --git a/Src/LexText/Interlinear/ITextDllTests/ComplexConcPatternVcDirectEditTests.cs b/Src/LexText/Interlinear/ITextDllTests/ComplexConcPatternVcDirectEditTests.cs new file mode 100644 index 0000000000..5eaa526577 --- /dev/null +++ b/Src/LexText/Interlinear/ITextDllTests/ComplexConcPatternVcDirectEditTests.cs @@ -0,0 +1,601 @@ +// 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.Collections.Generic; +using System.Reflection; +using System.Windows.Forms; +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; +using FS = System.Collections.Generic.Dictionary; + +namespace SIL.FieldWorks.IText +{ + /// + /// Covers the Complex Concordance pattern builder's response to an edit that reaches the + /// view engine without passing through PatternView.OnKeyPress, as IME composition and + /// drag-and-drop do. Without an UpdateProp override such an edit falls through to + /// VwBaseVc.UpdateProp, which throws. The pattern builder binds no real domain fields, so + /// the failure is a crash rather than a silent rename of shared project data. + /// + /// These tests drive a real IVwRootBox against an in-memory LcmCache and call + /// IVwSelection.ReplaceWithTsString directly. + /// + [TestFixture] + public class ComplexConcPatternVcDirectEditTests : 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(); + } + + 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; + } + + private class TestPatternView : PatternView + { + public void CallLayout() + { + OnLayout(new LayoutEventArgs(this, string.Empty)); + } + + public void SimulateKeyPress(char ch) + { + OnKeyPress(new KeyPressEventArgs(ch)); + } + + public void SimulateKeyDown(Keys key) + { + OnKeyDown(new KeyEventArgs(key)); + } + + public bool TestAllowDisplaySelection => AllowDisplaySelection; + } + + private IPartOfSpeech CreatePartOfSpeech(string name, string abbr) + { + IPartOfSpeech pos = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + if (Cache.LangProject.PartsOfSpeechOA == null) + Cache.LangProject.PartsOfSpeechOA = Cache.ServiceLocator.GetInstance().Create(); + pos = Cache.ServiceLocator.GetInstance().Create(); + Cache.LangProject.PartsOfSpeechOA.PossibilitiesOS.Add(pos); + pos.Name.SetAnalysisDefaultWritingSystem(name); + pos.Abbreviation.SetAnalysisDefaultWritingSystem(abbr); + }); + return pos; + } + + private ICmPossibility CreateTag(string name, string abbr) + { + ICmPossibility tag = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + if (Cache.LangProject.TextMarkupTagsOA == null) + Cache.LangProject.TextMarkupTagsOA = Cache.LangProject.GetDefaultTextTagList(); + tag = Cache.ServiceLocator.GetInstance().Create(); + Cache.LangProject.TextMarkupTagsOA.PossibilitiesOS.Add(tag); + tag.Name.SetAnalysisDefaultWritingSystem(name); + tag.Abbreviation.SetAnalysisDefaultWritingSystem(abbr); + }); + return tag; + } + + private IFsClosedFeature CreateClosedFeature(string name, out IFsSymFeatVal value) + { + IFsClosedFeature feat = null; + IFsSymFeatVal val = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + IFsFeatureSystem featSys = Cache.LanguageProject.MsFeatureSystemOA; + feat = Cache.ServiceLocator.GetInstance().Create(); + featSys.FeaturesOC.Add(feat); + feat.Name.SetAnalysisDefaultWritingSystem(name); + feat.Abbreviation.SetAnalysisDefaultWritingSystem(name); + val = Cache.ServiceLocator.GetInstance().Create(); + feat.ValuesOC.Add(val); + val.Name.SetAnalysisDefaultWritingSystem("v1"); + val.Abbreviation.SetAnalysisDefaultWritingSystem("v1"); + }); + value = val; + return feat; + } + + /// + /// Builds a live PatternView/ComplexConcPatternVc pair over a one-child pattern and + /// returns the view plus the model root (so callers can add children before use). + /// + private (ComplexConcPatternModel model, TestPatternView view) BuildView() + { + var model = new ComplexConcPatternModel(Cache); + var vc = new ComplexConcPatternVc(Cache, m_propertyTable); + var view = new TestPatternView { Cache = Cache, Visible = false, Width = 300, Height = 60 }; + view.Init(m_mediator, m_propertyTable, model.Root.Hvo, new NullPatternControl(), vc, + ComplexConcPatternVc.kfragPattern, model.DataAccess); + m_view = view; + return (model, view); + } + + /// + /// Selects the fragment rendered under fake tag on the child + /// node at , via IVwRootBox.MakeTextSelection with + /// passed as tagTextProp -- the same call PatternView itself uses + /// (PatternView.SelectLeftBoundary/SelectRightBoundary) to target one specific fake-tag + /// property on an object, as opposed to the whole object's rendering. + /// + /// MakeTextSelInObj (the API the original version of this helper used) does NOT take a + /// tag argument at all: its signature is (ihvoRoot, cvsli, rgvsli, cvsliEnd, rgvsliEnd, + /// fInitial, fEdit, fRange, fWholeObj, fInstall). The previous version of this helper + /// passed `tag` into the cvsliEnd slot and set fWholeObj: true, which per the documented + /// contract (Views.cs "If fWholeObject is true, these arguments are not used") made that + /// argument dead and made every call select the same thing: the whole child object's + /// rendering, anchored at its outermost boundary glyph. Nine of the eleven "fragment + /// angle" tests that used it were therefore the same selection in disguise -- see + /// MakeSelOnFragment_DiscriminatesBetweenFragments_OnTheSameNode below for the proof this + /// version actually targets the requested tag. + /// + private static IVwSelection MakeSelOnFragment(TestPatternView view, int childIndex, int tag, int ich = 0) + { + var levels = new[] + { + new SelLevInfo { tag = ComplexConcPatternSda.ktagChildren, ihvo = childIndex } + }; + return view.RootBox.MakeTextSelection(0, levels.Length, levels, tag, 0, ich, ich, 0, false, -1, null, true); + } + + /// + /// Fails the test with a diagnostic if does not actually target + /// -- proof, not assumption, that a fragment test is + /// exercising the fragment it claims to. + /// + private static void AssertSelectionTargets(IVwSelection sel, int expectedTag) + { + ITsString tss; + int ich, hvo, tag, ws; + bool fAssocPrev; + sel.TextSelInfo(false, out tss, out ich, out fAssocPrev, out hvo, out tag, out ws); + Assert.That(tag, Is.EqualTo(expectedTag), + $"selection did not target the requested tag {expectedTag}; got tag {tag} (text '{tss?.Text}')"); + } + + private void AttemptEdit(IVwSelection sel, string replacementText, int ws) + { + ITsString replacement = TsStringUtils.MakeString(replacementText, ws); + UndoableUnitOfWorkHelper.Do("undo", "redo", Cache.LangProject, () => sel.ReplaceWithTsString(replacement)); + } + + /// + /// Proof that MakeSelOnFragment actually discriminates between fragments, rather than + /// resolving to the same selection regardless of the requested tag (the bug an + /// adversarial review found in this helper's first version, which used + /// IVwRootBox.MakeTextSelInObj with fWholeObj: true -- see MakeSelOnFragment's doc + /// comment). One node with three populated fields is selected by three different tags; + /// each selection must land on its own distinct tag and text, not all collapse onto one + /// (e.g. the node's outermost boundary glyph). + /// + [Test] + public void MakeSelOnFragment_DiscriminatesBetweenFragments_OnTheSameNode() + { + var (model, view) = BuildView(); + var wordNode = new ComplexConcWordNode + { + Form = TsStringUtils.MakeString("myform", Cache.DefaultVernWs), + Gloss = TsStringUtils.MakeString("myGloss", Cache.DefaultAnalWs) + }; + model.Root.Children.Add(wordNode); + view.CallLayout(); + + IVwSelection selType = MakeSelOnFragment(view, 0, ComplexConcPatternVc.ktagType); + IVwSelection selForm = MakeSelOnFragment(view, 0, ComplexConcPatternVc.ktagForm); + IVwSelection selGloss = MakeSelOnFragment(view, 0, ComplexConcPatternVc.ktagGloss); + + AssertSelectionTargets(selType, ComplexConcPatternVc.ktagType); + AssertSelectionTargets(selForm, ComplexConcPatternVc.ktagForm); + AssertSelectionTargets(selGloss, ComplexConcPatternVc.ktagGloss); + + ITsString tss; int ich, hvo, tag, ws; bool fAssocPrev; + selType.TextSelInfo(false, out tss, out ich, out fAssocPrev, out hvo, out tag, out ws); + Assert.That(tss.Text, Is.EqualTo("Type: Word")); + selForm.TextSelInfo(false, out tss, out ich, out fAssocPrev, out hvo, out tag, out ws); + Assert.That(tss.Text, Is.EqualTo("Form: myform")); + selGloss.TextSelInfo(false, out tss, out ich, out fAssocPrev, out hvo, out tag, out ws); + Assert.That(tss.Text, Is.EqualTo("Gloss: myGloss")); + } + + // Breadth of the crash across every fragment ComplexConcPatternVc renders. Each case + // asserts no crash and unchanged content. + + [Test] + public void ReplaceWithTsString_OnWordNodeTypeLine_DoesNotThrow() + { + var (model, view) = BuildView(); + var wordNode = new ComplexConcWordNode(); + model.Root.Children.Add(wordNode); + view.CallLayout(); + + IVwSelection sel = MakeSelOnFragment(view, 0, ComplexConcPatternVc.ktagType); + Assert.That(sel, Is.Not.Null, "could not construct a selection over the Type line"); + AssertSelectionTargets(sel, ComplexConcPatternVc.ktagType); + + Assert.DoesNotThrow(() => AttemptEdit(sel, "HACKED", Cache.DefaultUserWs), + "a direct edit on the word node's Type line must not crash the view engine"); + } + + [Test] + public void ReplaceWithTsString_OnWordNodeFormLine_DoesNotThrow_AndFormUnchanged() + { + var (model, view) = BuildView(); + var wordNode = new ComplexConcWordNode { Form = TsStringUtils.MakeString("original", Cache.DefaultVernWs) }; + model.Root.Children.Add(wordNode); + view.CallLayout(); + + IVwSelection sel = MakeSelOnFragment(view, 0, ComplexConcPatternVc.ktagForm); + Assert.That(sel, Is.Not.Null, "could not construct a selection over the Form line"); + AssertSelectionTargets(sel, ComplexConcPatternVc.ktagForm); + + Assert.DoesNotThrow(() => AttemptEdit(sel, "HACKED", Cache.DefaultVernWs), + "a direct edit on the word node's Form line must not crash the view engine"); + Assert.That(wordNode.Form.Text, Is.EqualTo("original"), + "the synthetic pattern node's Form must not be mutated by a discarded edit"); + } + + [Test] + public void ReplaceWithTsString_OnMorphNodeGlossLine_DoesNotThrow_AndGlossUnchanged() + { + var (model, view) = BuildView(); + var morphNode = new ComplexConcMorphNode { Gloss = TsStringUtils.MakeString("original-gloss", Cache.DefaultAnalWs) }; + model.Root.Children.Add(morphNode); + view.CallLayout(); + + IVwSelection sel = MakeSelOnFragment(view, 0, ComplexConcPatternVc.ktagGloss); + Assert.That(sel, Is.Not.Null, "could not construct a selection over the Gloss line"); + AssertSelectionTargets(sel, ComplexConcPatternVc.ktagGloss); + + Assert.DoesNotThrow(() => AttemptEdit(sel, "HACKED", Cache.DefaultAnalWs), + "a direct edit on the morph node's Gloss line must not crash the view engine"); + Assert.That(morphNode.Gloss.Text, Is.EqualTo("original-gloss"), + "the synthetic pattern node's Gloss must not be mutated by a discarded edit"); + } + + [Test] + public void ReplaceWithTsString_OnMorphNodeEntryLine_DoesNotThrow_AndEntryUnchanged() + { + var (model, view) = BuildView(); + var morphNode = new ComplexConcMorphNode { Entry = TsStringUtils.MakeString("original-entry", Cache.DefaultVernWs) }; + model.Root.Children.Add(morphNode); + view.CallLayout(); + + IVwSelection sel = MakeSelOnFragment(view, 0, ComplexConcPatternVc.ktagEntry); + Assert.That(sel, Is.Not.Null, "could not construct a selection over the Entry line"); + AssertSelectionTargets(sel, ComplexConcPatternVc.ktagEntry); + + Assert.DoesNotThrow(() => AttemptEdit(sel, "HACKED", Cache.DefaultVernWs), + "a direct edit on the morph node's Entry line must not crash the view engine"); + Assert.That(morphNode.Entry.Text, Is.EqualTo("original-entry"), + "the synthetic pattern node's Entry must not be mutated by a discarded edit"); + } + + [Test] + public void ReplaceWithTsString_OnMorphNodeCategoryLine_DoesNotThrow_AndRealPartOfSpeechUnrenamed() + { + IPartOfSpeech noun = CreatePartOfSpeech("noun", "N"); + var (model, view) = BuildView(); + var morphNode = new ComplexConcMorphNode { Category = noun }; + model.Root.Children.Add(morphNode); + view.CallLayout(); + + IVwSelection sel = MakeSelOnFragment(view, 0, ComplexConcPatternVc.ktagCategory); + Assert.That(sel, Is.Not.Null, "could not construct a selection over the Category line"); + AssertSelectionTargets(sel, ComplexConcPatternVc.ktagCategory); + + Assert.DoesNotThrow(() => AttemptEdit(sel, "HACKED", Cache.DefaultAnalWs), + "a direct edit on the morph node's Category line must not crash the view engine"); + // The category line displays a real, shared IPartOfSpeech's Abbreviation, so a + // botched edit here would rename it project-wide. + Assert.That(noun.Abbreviation.BestAnalysisAlternative.Text, Is.EqualTo("N"), + "an edit attempt on the Category line must not rename the real, shared PartOfSpeech"); + } + + [Test] + public void ReplaceWithTsString_OnMorphNodeInflLine_DoesNotThrow() + { + IFsSymFeatVal value; + IFsClosedFeature feature = CreateClosedFeature("num", out value); + var (model, view) = BuildView(); + var morphNode = new ComplexConcMorphNode + { + InflFeatures = { { feature, new ClosedFeatureValue(value, false) } } + }; + model.Root.Children.Add(morphNode); + view.CallLayout(); + + IVwSelection sel = MakeSelOnFragment(view, 0, ComplexConcPatternVc.ktagInfl); + Assert.That(sel, Is.Not.Null, "could not construct a selection over the Infl Features header line"); + AssertSelectionTargets(sel, ComplexConcPatternVc.ktagInfl); + + Assert.DoesNotThrow(() => AttemptEdit(sel, "HACKED", Cache.DefaultAnalWs), + "a direct edit on the morph node's Infl Features header line must not crash the view engine"); + } + + [Test] + public void ReplaceWithTsString_OnTagNodeTagLine_DoesNotThrow_AndRealTagUnrenamed() + { + ICmPossibility tag = CreateTag("Noun Phrase", "NP"); + var (model, view) = BuildView(); + var tagNode = new ComplexConcTagNode { Tag = tag }; + model.Root.Children.Add(tagNode); + view.CallLayout(); + + IVwSelection sel = MakeSelOnFragment(view, 0, ComplexConcPatternVc.ktagTag); + Assert.That(sel, Is.Not.Null, "could not construct a selection over the Tag line"); + AssertSelectionTargets(sel, ComplexConcPatternVc.ktagTag); + + Assert.DoesNotThrow(() => AttemptEdit(sel, "HACKED", Cache.DefaultAnalWs), + "a direct edit on the tag node's Tag line must not crash the view engine"); + Assert.That(tag.Abbreviation.BestAnalysisAlternative.Text, Is.EqualTo("NP"), + "an edit attempt on the Tag line must not rename the real, shared CmPossibility"); + } + + [Test] + public void ReplaceWithTsString_OnOrNode_DoesNotThrow() + { + var (model, view) = BuildView(); + model.Root.Children.Add(new ComplexConcOrNode()); + model.Root.Children.Add(new ComplexConcWordBdryNode()); + view.CallLayout(); + + IVwSelection sel = MakeSelOnFragment(view, 0, PatternVcBase.ktagInnerNonBoundary); + Assert.That(sel, Is.Not.Null, "could not construct a selection over the OR literal"); + AssertSelectionTargets(sel, PatternVcBase.ktagInnerNonBoundary); + + Assert.DoesNotThrow(() => AttemptEdit(sel, "HACKED", Cache.DefaultUserWs), + "a direct edit on the OR literal must not crash the view engine"); + } + + [Test] + public void ReplaceWithTsString_OnWordBoundaryNode_DoesNotThrow() + { + var (model, view) = BuildView(); + model.Root.Children.Add(new ComplexConcOrNode()); + model.Root.Children.Add(new ComplexConcWordBdryNode()); + view.CallLayout(); + + IVwSelection sel = MakeSelOnFragment(view, 1, PatternVcBase.ktagInnerNonBoundary); + Assert.That(sel, Is.Not.Null, "could not construct a selection over the '#' literal"); + AssertSelectionTargets(sel, PatternVcBase.ktagInnerNonBoundary); + + Assert.DoesNotThrow(() => AttemptEdit(sel, "HACKED", Cache.DefaultUserWs), + "a direct edit on the word-boundary '#' literal must not crash the view engine"); + } + + [Test] + public void ReplaceWithTsString_OnNodeMaximum_DoesNotThrow_AndMaximumUnchanged() + { + var (model, view) = BuildView(); + var wordNode = new ComplexConcWordNode { Minimum = 0, Maximum = 3 }; + model.Root.Children.Add(wordNode); + view.CallLayout(); + + IVwSelection sel = MakeSelOnFragment(view, 0, PatternVcBase.ktagRightNonBoundary); + Assert.That(sel, Is.Not.Null, "could not construct a selection over the max-quantifier line"); + AssertSelectionTargets(sel, PatternVcBase.ktagRightNonBoundary); + + Assert.DoesNotThrow(() => AttemptEdit(sel, "9", Cache.DefaultUserWs), + "a direct edit on the max-quantifier line must not crash the view engine"); + Assert.That(wordNode.Maximum, Is.EqualTo(3), + "the synthetic pattern node's Maximum must not be mutated by a discarded edit"); + } + + [Test] + public void ReplaceWithTsString_OnNodeMinimum_DoesNotThrow_AndMinimumUnchanged() + { + var (model, view) = BuildView(); + var wordNode = new ComplexConcWordNode { Minimum = 0, Maximum = 3 }; + model.Root.Children.Add(wordNode); + view.CallLayout(); + + IVwSelection sel = MakeSelOnFragment(view, 0, PatternVcBase.ktagRightBoundary); + Assert.That(sel, Is.Not.Null, "could not construct a selection over the min-quantifier line"); + AssertSelectionTargets(sel, PatternVcBase.ktagRightBoundary); + + Assert.DoesNotThrow(() => AttemptEdit(sel, "9", Cache.DefaultUserWs), + "a direct edit on the min-quantifier line must not crash the view engine"); + Assert.That(wordNode.Minimum, Is.EqualTo(0), + "the synthetic pattern node's Minimum must not be mutated by a discarded edit"); + } + + // Ordinary typing never reaches the view engine: OnKeyPress handles everything except + // Backspace and Delete, so only paths that bypass it can crash. + + [Test] + public void SimulateTyping_ViaOnKeyPress_DoesNotReachEngine_AndDoesNotCrash() + { + var (model, view) = BuildView(); + var wordNode = new ComplexConcWordNode { Form = TsStringUtils.MakeString("original", Cache.DefaultVernWs) }; + model.Root.Children.Add(wordNode); + view.CallLayout(); + + IVwSelection sel = MakeSelOnFragment(view, 0, ComplexConcPatternVc.ktagForm); + Assert.That(sel, Is.Not.Null, "could not construct a selection over the Form line"); + sel.Install(); + + Assert.DoesNotThrow(() => view.SimulateKeyPress('x'), + "a plain keystroke must not crash PatternView, regardless of the ComplexConcPatternVc.UpdateProp gap"); + Assert.That(wordNode.Form.Text, Is.EqualTo("original"), + "a plain keystroke must not reach the engine and alter content -- PatternView.OnKeyPress swallows it before that"); + } + + // Insert and delete must keep working: OnKeyDown raises RemoveItemsRequested for Delete. + + // ------------------------------------------------------------------ + // Ablation evidence for the fix's layers. + // ------------------------------------------------------------------ + + /// + /// Confirms ComplexConcPatternVc's SetNotEditable calls actually take effect: a + /// selection over a fake-tag fragment must not be editable, independent of whether + /// UpdateProp would otherwise absorb an edit there. + /// + [Test] + public void SelectionOverFormLine_IsNotEditable() + { + var (model, view) = BuildView(); + var wordNode = new ComplexConcWordNode { Form = TsStringUtils.MakeString("original", Cache.DefaultVernWs) }; + model.Root.Children.Add(wordNode); + view.CallLayout(); + + IVwSelection sel = MakeSelOnFragment(view, 0, ComplexConcPatternVc.ktagForm); + Assert.That(sel, Is.Not.Null); + AssertSelectionTargets(sel, ComplexConcPatternVc.ktagForm); + Assert.That(sel.IsEditable, Is.False, + "the Form line's fragment must be marked ktptNotEditable, not merely absorbed by UpdateProp"); + } + + /// + /// PatternVcBase.OpenSingleLinePile/CloseSingleLinePile add a 1-char zero-width-space + /// boundary run (tag ktagLeftBoundary/ktagRightBoundary, frag kfragZeroWidthSpace) around + /// single-line piles, with no ktptEditable marking at all -- an unmarked gap shared by + /// both PatternVcBase subclasses, found by mutation testing: with UpdateProp removed and + /// every other guard left in place, an edit on this run still threw + /// NotImplementedException, because nothing else was rejecting it at the selection layer. + /// This test fails without the ktptEditable marking added to OpenSingleLinePile/ + /// CloseSingleLinePile (verified by temporarily reverting it) and passes with it. + /// + [Test] + public void SelectionOverZeroWidthBoundaryRun_IsNotEditable() + { + var (model, view) = BuildView(); + model.Root.Children.Add(new ComplexConcOrNode()); + view.CallLayout(); + + // The 1-char ZWSP boundary run precedes OR in the same paragraph, at ich=0. + IVwSelection sel = MakeSelOnFragment(view, 0, PatternVcBase.ktagLeftBoundary, 0); + Assert.That(sel, Is.Not.Null, "could not construct a selection over the zero-width-space boundary run"); + AssertSelectionTargets(sel, PatternVcBase.ktagLeftBoundary); + Assert.That(sel.IsEditable, Is.False, + "the zero-width-space boundary run (PatternVcBase.OpenSingleLinePile) must be marked ktptNotEditable"); + } + + /// + /// ComplexConcControl.Designer.cs must wire up the pattern-builder view as read-only + /// (this is the categorical fix for the IME-composition/keyboard-controller-registration + /// path named in the bug report, distinct from the per-fragment ktptEditable markings). + /// + [Test] + public void ComplexConcControl_WiresViewAsReadOnly() + { + using (var control = new ComplexConcControl()) + { + var viewField = typeof(ComplexConcControl).GetField("m_view", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.That(viewField, Is.Not.Null, "test assumption: ComplexConcControl has a private m_view field"); + var view = (PatternView) viewField.GetValue(control); + Assert.That(view.ReadOnlyView, Is.True, + "ComplexConcControl must wire up its PatternView with ReadOnlyView = true"); + } + } + + /// + /// SimpleRootSite.ReadOnlyView's setter forces AcceptsReturn and AcceptsTab to false, + /// after the Designer's own assignments in InitializeComponent. AcceptsTab is false + /// either way, so Tab navigation out of the pane is unchanged. AcceptsReturn becomes + /// false, and since PatternView.OnKeyPress swallows Return regardless, the only + /// difference is where the key is disposed of: IsInputKey(Return) returns false, so the + /// key is not delivered to the control and whatever hosts the pane treats it as an + /// ordinary navigation key. The pane is a Words-area tool rather than a modal dialog + /// with an AcceptButton, so no default-button activation is expected. Unverified live. + /// + [Test] + public void ComplexConcControl_AcceptsTabUnchanged_AcceptsReturnNowFalse() + { + using (var control = new ComplexConcControl()) + { + var viewField = typeof(ComplexConcControl).GetField("m_view", BindingFlags.NonPublic | BindingFlags.Instance); + var view = (PatternView) viewField.GetValue(control); + + Assert.That(view.AcceptsTab, Is.False, + "AcceptsTab was already false in the Designer before this fix; it must still be false"); + Assert.That(view.AcceptsReturn, Is.False, + "ReadOnlyView = true forces AcceptsReturn to false, overriding the Designer's " + + "explicit AcceptsReturn = true -- this is the real behaviour change, not Tab"); + } + } + + /// + /// PatternView.AllowDisplaySelection must be overridden to stay true even when + /// ReadOnlyView is true, or the pattern-builder's chooser-driven selection highlight + /// would disappear once ReadOnlyView is turned on (SimpleRootSite suppresses Activate() + /// by default for read-only views). + /// + [Test] + public void AllowDisplaySelection_IsTrue_WhenRootsiteIsReadOnly() + { + var (model, view) = BuildView(); + view.ReadOnlyView = true; + + Assert.That(view.TestAllowDisplaySelection, Is.True, + "the selection must still be shown even though the rootsite is read-only"); + } + + [Test] + public void DeleteKey_StillRaisesRemoveItemsRequested_WhenRootsiteIsReadOnly() + { + var (model, view) = BuildView(); + model.Root.Children.Add(new ComplexConcWordNode()); + view.CallLayout(); + view.ReadOnlyView = true; + + bool removeRequested = false; + view.RemoveItemsRequested += (sender, e) => removeRequested = true; + + view.SimulateKeyDown(Keys.Delete); + + Assert.That(removeRequested, Is.True, + "Delete must still raise RemoveItemsRequested now that ComplexConcControl wires the view as ReadOnlyView = true"); + } + } +} diff --git a/Src/LexText/LexTextControls/PatternVcBase.cs b/Src/LexText/LexTextControls/PatternVcBase.cs index ad4bd25fb7..e8d1c383d4 100644 --- a/Src/LexText/LexTextControls/PatternVcBase.cs +++ b/Src/LexText/LexTextControls/PatternVcBase.cs @@ -226,7 +226,10 @@ protected void OpenSingleLinePile(IVwEnv vwenv, int maxNumLines, bool addBoundar vwenv.OpenParagraph(); if (addBoundary) { + // This zero-width-space boundary run is a cursor-parking glyph for arrow-key + // navigation, never free text; a fake tag with no bound field behind it. vwenv.Props = m_bracketProps; + vwenv.set_IntProperty((int) FwTextPropType.ktptEditable, (int) FwTextPropVar.ktpvEnum, (int) TptEditable.ktptNotEditable); vwenv.AddProp(ktagLeftBoundary, this, kfragZeroWidthSpace); } } @@ -241,6 +244,7 @@ protected void CloseSingleLinePile(IVwEnv vwenv, bool addBoundary) if (addBoundary) { vwenv.Props = m_bracketProps; + vwenv.set_IntProperty((int) FwTextPropType.ktptEditable, (int) FwTextPropVar.ktpvEnum, (int) TptEditable.ktptNotEditable); vwenv.AddProp(ktagRightBoundary, this, kfragZeroWidthSpace); } vwenv.CloseParagraph(); diff --git a/Src/LexText/LexTextControls/PatternView.cs b/Src/LexText/LexTextControls/PatternView.cs index 27bbb76eca..571ba32d61 100644 --- a/Src/LexText/LexTextControls/PatternView.cs +++ b/Src/LexText/LexTextControls/PatternView.cs @@ -69,6 +69,19 @@ protected override EditingHelper CreateEditingHelper() return new PatternEditingHelper(Cache, this); } + /// + /// Activate() is suppressed by default in ReadOnlyViews + /// (SimpleRootSite.AllowDisplaySelection + /// defaults to IsEditable), but both PatternView consumers are pattern builders whose + /// chooser + /// insert/delete needs the user to see the current selection even when the view itself is + /// read-only. + /// + 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();