LT-22710: Make phonological rule formula cells non-editable

[johnml1135]

Typing into a phonological rule formula no longer changes anything. Until now a rule cell rendered the referenced phoneme's or natural class's own Name/Abbreviation as editable text, so an edit that reached the view renamed that object across the entire project — every other rule referencing it changed with it.

The question on opening a diff with four production lines and three hundred test lines is why the tests outweigh the fix. Because the invariant was previously held by a keystroke filter — PatternView.OnKeyPress, itself added for LT-21888 — that covers exactly one input path. This branch moves enforcement into the view constructor, where nothing can route around it, and pins each layer with a test that goes red when that layer alone is removed. The review worth your time is whether a read-only rootsite breaks an interaction users still need, not whether the markings are right.

Where to look

• Deletion still has to work with the rootsite read-only — DeleteKey_StillRaisesRemoveItemsRequested_WhenRootsiteIsReadOnly pins it.
• ReadOnlyView = true is a broad switch (selection, focus, IME registration, cut/paste). AllowDisplaySelection restores visible selection; three per-control tests pin the shipped wiring.
• PatternView is shared with the Complex Concordance pattern builder, which still runs editable. Nothing here alters its behaviour.
• All three rule kinds inherit the fix from RuleFormulaVcBase; regular, metathesis and affix-process each have their own test.
• OnKeyPress and PatternEditingHelper.CanCut/CanPaste are deliberately kept — still load-bearing for the other consumer.

Deliberately not here

• Live IME / drag-and-drop verification in a running FLEx. The corruption is proven by test through ReplaceWithTsString; the user-facing input path is not.
• ComplexConcPatternVc has a sibling defect — a crash, not corruption. Separate branch.
• ConstChartVc appears guarded at cell level. Assessed by reading only.

One side effect worth knowing. ReadOnlyView = true also forces AcceptsReturn to false (SimpleRootSite.cs:942). PatternView ships Tab-false / Return-true, so Tab is unaffected and Return changes only in where it is disposed of — OnKeyPress already swallowed it, and it now propagates to the host instead of being trapped. The rule formula is a DataTree slice, not a dialog with an AcceptButton, so this should be inert. Reasoned, not verified.

Verification. MorphologyEditorDllTests 21/21, LexTextControlsTests 353 passed / 3 skipped, ITextDllTests 207 passed / 1 skipped. Against the test-only commit the reproduction suite is 5 failed / 0 passed. Not stacked.

---

Reading this a year from now — start here

The investigation notes and architecture review that produced this fix were deliberately deleted from the branch rather than merged; their content is synthesised into these sections. They were working documents — a one-time analysis whose conclusions are now carried by the code and its tests.

The short version: a rule formula cell is not free text, and never was meant to be. That invariant lived in exactly one place — a WM_CHAR filter — and anything reaching the root box by another route (IME composition being the realistic case for vernacular keyboards) walked straight past it into a shared domain object's name field.

The layer cake

Rule formula editing, model outward:

Layer	Type
Model	PhSegmentRule/PhRegularRule → PhSegRuleRHS cells → PhPhonContext trees
View constructor	RuleFormulaVcBase ← PatternVcBase ← FwBaseVc
Rootsite	PatternView
Control	RuleFormulaControl + RegRule/AffixRule/MetaRule subclasses
Insert UI	InsertionControl — a hotlink strip, not a menu
Slice	RuleFormulaSlice

A "cell" is StrucDesc/StrucChange/LeftContext/RightContext on a PhSegRuleRHS. kfragNC and kfragTerminalUnit render the referenced PhNaturalClass's and PhTerminalUnit's real multistring properties — which is why an edit renamed the object rather than corrupting the rule.

PatternVcBase has exactly two subclasses and PatternView exactly two consumers, so the audit surface here is closed, not open-ended.

Decisions, and why

Enforce in the view constructor, not with another input filter. The obvious cheap fix is to intercept whatever new path let the edit through. That is how the bug got here: OnKeyPress was already such a fix. Marking fragments ktptNotEditable denies the edit at the layer that owns the question, so a future input path inherits the guard instead of needing its own.

Both layers were kept because both are load-bearing, established by ablation rather than assumed. Removing just the kfragTerminalUnit marking while leaving ReadOnlyView = true reproduces the corruption. So the fragment markings — not ReadOnlyView — are what actually block ReplaceWithTsString. ReadOnlyView does separate work: it unregisters the keyboard/IME controller hook and disables cut/paste. Neither alone is sufficient.

AllowDisplaySelection follows the existing InterlinPrintView/InterlinTaggingChild pattern. A read-only rootsite suppresses selection display by default, but a pattern editor needs the user to see what an insert or delete will act on.

Paths not taken

Removing OnKeyPress as now-redundant. It is not. An audit found bracket-glyph and fake-tag spans in MetaRuleFormulaVc, AffixRuleFormulaVc and PatternVcBase for which it remains the only guard. Marking those is out of scope here.

Removing PatternEditingHelper.CanCut/CanPaste. They look dead once the rootsite is read-only, but PatternView is shared with ComplexConcControl, which as of this branch still runs with ReadOnlyView = false. Deleting them here would silently enable paste there.

Update: the Complex Concordance branch does set ReadOnlyView = true, so once both land these two overrides become provably dead — SimpleRootSite's ReadOnlyView setter is EditingHelper.Editable = !value (SimpleRootSite.cs:907-944), and base EditingHelper.CanCut/CanPaste (EditingHelper.cs:3672, 3789) already return false when m_fEditable is false, with no dependence on the override. CanCopy is unaffected either way. Removing them is deliberately left to a follow-up after both branches are in, rather than making this branch depend on one that is not yet open.

Fixing ComplexConcPatternVc in the same branch. Different feature area, and a different failure mode: it binds no real domain fields, so it cannot reproduce this rename. It throws instead.

Surprising findings

The corruption is real and reproducible, not theoretical. The original report described it as "sometimes the user can modify the underlying forms." A test now drives IVwSelection.ReplaceWithTsString on a phoneme terminal unit and the real PhPhoneme.Name comes back as "CORRUPTED".

ReadOnlyView does not gate the low-level edit API. Setting it alone leaves ReplaceWithTsString working. This was measured, not reasoned about, and it is the reason the fix has two layers.

Three of the five fragment markings were initially untested — they could be deleted with the suite green. Found by mutation testing after the first implementation and closed since; the current suite catches each one individually.

Evidence

Each claim in "Where to look" is pinned by a mutation, run rather than argued:

Mutation	Result
Delete kfragTerminalUnit marking	4 tests red, incl. PhPhoneme.Name = "CORRUPTED"
Delete kfragNC marking	mock-level and end-to-end natural-class tests red
Delete kfragFeature / kfragPlusVariable / kfragMinusVariable	corresponding fragment tests red
Flip ReadOnlyView back to false	3 per-control wiring tests red, all three rule kinds
Remove AllowDisplaySelection	its test red, alone

The EditabilityRecordingEnv spy was extended to record AddProp (it previously threw), which is what the feature and variable fragments call.

Deferred, and what would unblock it

• Live IME verification. Needs a running FLEx with an active IME; not available in this environment. The mechanism is proven; the user path is inferred.
• ComplexConcPatternVc — sibling defect in the other PatternVcBase subclass, confirmed by probe: no UpdateProp override, so any bypassing edit throws an unhandled NotImplementedException. Own branch.
• ConstChartVc:297 binds a shared CmPossibility in a chooser-driven cell — same defect shape — but appears pre-guarded at the cell level by MakeCellsMethod.cs:495. Assessed by code reading only; never exercised.
• The base-class question. Both PatternVcBase subclasses have now had a bug in this family. Whether "pattern views are never free text" should be enforced in PatternVcBase/PatternView themselves, so a third subclass cannot reintroduce it, is worth deciding once the Complex Concordance fix lands.

---

This change is 

[github-actions]

Comment hygiene (advisory)

No comment-style violations in the lines this branch adds since origin/main.

[github-actions]

NUnit Tests

    1 files  ± 0      1 suites  ±0   10m 40s ⏱️ -1s
5 813 tests +14  5 732 ✅ +14  81 💤 ±0  0 ❌ ±0 
5 822 runs  +14  5 741 ✅ +14  81 💤 ±0  0 ❌ ±0 

Results for commit 4fe7c4c. ± Comparison against base commit c373152.

♻️ This comment has been updated with latest results.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 38.45%. Comparing base (c373152) to head (4fe7c4c).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1082      +/-   ##
==========================================
+ Coverage   38.28%   38.45%   +0.17%     
==========================================
  Files        1499     1499              
  Lines      350230   350236       +6     
  Branches    40261    40261              
==========================================
+ Hits       134096   134695     +599     
+ Misses     186915   186342     -573     
+ Partials    29219    29199      -20     

Files with missing lines	Coverage Δ
Src/LexText/LexTextControls/PatternView.cs	22.26% <100.00%> (+22.26%)	⬆️
Src/LexText/Morphology/RuleFormulaControl.cs	7.71% <100.00%> (+7.71%)	⬆️
Src/LexText/Morphology/RuleFormulaVcBase.cs	20.14% <100.00%> (+20.14%)	⬆️

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.