LT-22685: Add sense pictures to the legacy Lexicon Edit pane (split from #964)

[johnml1135]

Add sense pictures to the legacy Lexicon Edit pane

Jira: LT-22685

Split out of the Phase-1 Avalonia spine (#964) so the team can assess it on its own merits.

What it does

Adds a LexSense-Detail-Pictures part to LexSenseParts.xml. The legacy LexSense layout has referenced <part ref="Pictures"/> for years, but no LexSense-Detail-Pictures part existed to resolve it — so DataTree silently omitted the slice and sense pictures have never appeared in the Lexicon Edit pane. With the part present, senses that have pictures will now show the Picture / Caption / File / Publish-In slices and the mnuDataTree-Picture insert/delete menu (all of that scaffolding already exists and resolves; only the top-level part was missing).

Why it is its own PR

The parts inventory is shared across UIMode, so this is an ungated, legacy-visible behavior change — every user with sense pictures sees the new section, regardless of Legacy/New mode. It is very likely a desirable, long-intended bug-fix, but it changes what existing Legacy users see, so it should be a deliberate decision rather than a side effect of the migration spine.

Status / scope

• Draft, for assessment — not tied to the migration stack.
• Verified only for XML well-formedness here. If the team wants it, the natural follow-up is a legacy DataTree test asserting the slices appear + a before/after screenshot.
• Note: an earlier malformed version of this part (using editor="picture" on the sequence field) crashed the legacy DataTree; this uses the correct owned-sequence form (layout="Normal"), documented in the in-file comment.

---

This change is

[github-actions]

⚠️ Commit Message Format Issues ⚠️

commit 4531bec115:
1: T1 Title exceeds max length (79>72): "Add LexSense-Detail-Pictures part so sense pictures show in legacy Lexicon Edit"

[github-actions]

NUnit Tests

    1 files  ±0      1 suites  ±0   10m 16s ⏱️ -20s
4 302 tests ±0  4 229 ✅ ±0  73 💤 ±0  0 ❌ ±0 
4 311 runs  ±0  4 238 ✅ ±0  73 💤 ±0  0 ❌ ±0 

Results for commit 4531bec. ± Comparison against base commit 84a4850.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 32.99%. Comparing base (84a4850) to head (4531bec).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1012      +/-   ##
==========================================
- Coverage   32.99%   32.99%   -0.01%     
==========================================
  Files        1202     1202              
  Lines      278168   278168              
  Branches    37151    37151              
==========================================
- Hits        91781    91780       -1     
  Misses     158537   158537              
- Partials    27850    27851       +1     

see 1 file 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.

[jasonleenaylor]

The diagnosis is right and I verified it: LexSense.fwlayout:46 really does carry
<part ref="Pictures" param="Normal"/>, git log -S "LexSense-Detail-Pictures" returns
nothing, and CmdInsertPicture is live on mnuDataTree-Sense
(DataTreeInclude.xml:448) — so users can create sense pictures today and watch them
vanish. Rejecting editor="picture" was also correct: SliceFactory.cs:210 does a hard
(ICmPicture)obj cast and would receive the owning LexSense.

Two things you got right that are worth recording, because the next reader will
otherwise re-litigate them:

• No ghost is the correct call. DataTree.MakeGhostSlice (DataTree.cs:2800)
  always constructs a GhostStringSlice, whose contract is strict: ghostWs is
  mandatory (GhostStringSlice.cs:116) and MakeRealObject throws "ghost property must
  store strings!" unless the target field is String/MultiString/MultiUnicode
  (:305-318). A ghost on Pictures would therefore have to create a CmPicture from
  typed text — yielding a picture record with a caption and no PictureFileRA. Pictures
  need PicturePropertiesDialog to choose a file first (DTMenuHandler.cs:224-236),
  which a ghost cannot express. Worth one clause in the comment so nobody "fixes" this
  later.
• Senses with no pictures stay clean. DataTree.cs:2912 only adds a slice when
  cobj > 0 or a ghost is present, so there is no empty-row clutter.

What needs to change before this merges.

1. A DataTree test is a condition of this PR, not a follow-up.

This is the part I do not want to trade away. ref="Pictures" has exactly one referent
and no part has ever resolved it, so this three-line change is the switch that first
energises five CmPicture-Detail-* parts, PictureSlice, CmFile-Detail-FileName, and
a defaultVectorReference on PublishIn — roughly 35 lines of configuration that have
never executed in production, for every user with sense pictures, ungated. CI reporting
4302 tests identical to base confirms the suite is blind to it, and "verified only for
XML well-formedness" is not evidence for a change of that reach. You already crashed the
legacy DataTree in this exact area, which is the argument for the test rather than
against it.

A test asserting the five slices materialise for a sense with one picture is the
minimum. This is reachable through the DataTree slice-creation path without
constructing a Form, so the no-WinForms-dialog-tests rule is not in the way.

2. Drop layout="Normal".

DataTree.cs:2918 reads
layoutOverride = XmlUtils.GetOptionalAttributeValue(node, "layout", layoutName), where
layoutName is the caller's param. The layout node already says param="Normal", so
the attribute is a no-op today and a silent override tomorrow — any future
<part ref="Pictures" param="Something"/> would be ignored. The live exemplar,
LexSense-Detail-Examples (LexSenseParts.xml:620-623), omits it and lets param flow
through.

3. Fix or drop menu="mnuDataTree-Picture".

Two problems. It is the wrong menu: DataTreeInclude.xml:403-410 shows
mnuDataTree-Picture holds Properties / MoveUp / MoveDown / Delete and no insert
command, so the body's "insert/delete menu" is inaccurate. The established idiom for a
seq node is the plural sequence-level menu — mnuDataTree-Examples
(DataTreeInclude.xml:468) and mnuDataTree-ExtendedNotes (:500) both hold the insert
command — and no mnuDataTree-Pictures exists.

It also probably never fires: DTMenuHandler.MakeSliceContextMenu
(DTMenuHandler.cs:1708-1716) reads the menu id off slice.CallerNode /
slice.ConfigurationNode, and with no ghost this seq node becomes neither. Each
CmPicture-Detail-* slice already carries its own menu="mnuDataTree-Picture"
(LexSenseParts.xml:887, 903, 906, 909). Either add a mnuDataTree-Pictures containing
CmdInsertPicture, or drop the attribute — but do not leave a dead attribute that
implies an insert affordance that is not there.

4. Rewrite the comment.

Six lines and roughly 570 characters, against a target of 3-4 sentences for a doc
comment and 200 characters for an implementation comment. Every line exceeds the
98-column limit from .editorconfig (102, 105, 107, 105, 97, 107), and line 3 contains a
literal em dash where the standard is ASCII. .xml sits outside
comment-hygiene.ps1's globs so nothing caught it mechanically, but the convention still
applies.

Content problems, in order of consequence:

• It cites a dead exemplar. LexSense-Detail-ExampleList (:60-62) exists but nothing
  in the repo references ref="ExampleList". The part that actually resolves is
  LexSense-Detail-Examples (:620), and it uses a different form.
• It misquotes the layout node it explains: the node is
  <part ref="Pictures" param="Normal"/>, not <part ref="Pictures"/>. The dropped
  attribute is exactly the one that makes item 2 above a finding. The same misquote is in
  the PR body and the commit message.
• "the malformed form this replaces" describes a version that never existed in the
  repository, and "silently omitted there for years because no LexSense-Detail-Pictures
  part existed" narrates what was missing rather than what the part does.
• Two of the four sentences narrate the cast mechanics of the rejected alternative. That
  warning is genuinely valuable — keep it as one sentence ("Do not use
  editor="picture" here; the slice factory casts the owning object to ICmPicture.")
  rather than a paragraph.

Roughly: what the part renders, the editor="picture" trap, and the ghost note from
above.

5. The body understates the blast radius, and the Avalonia gap needs a Jira record.

The body calls this "an ungated, legacy-visible behavior change." It is not legacy-only.
DetailComposer.cs:950-954 routes DetailEditorCategory.Picture to WalkUnsupported,
so the moment this part resolves, every sense with a picture grows a labeled
"Unsupported" row in the Avalonia UI. Legacy users get an image; new-UI users get a
worklist row they did not have before.

Fixing DetailComposer is out of scope here — that is the migration work this split was
designed to escape. But please correct the body to say both modes, and record the picture
gap as a known divergence in the Avalonia conversion's Jira issue so it is tracked before
it ships rather than discovered by a user.

6. Add LabelAbbreviations entries for the newly-visible labels.

strings-en.xml group LabelAbbreviations (line 367) carries an abbreviation for every
routinely-visible sense field ("Scientific Name" txt="sc", line 427). "Picture",
"Caption", "File", and "Publish Picture In" appear nowhere in strings-en.xml, so in
abbreviated-label mode these rows fall back to raw English. No hardcoded strings were
added by this PR, so nothing is violated — the catalog is just incomplete for the surface
this exposes.

7. Commit title is 79 characters against gitlint's 72.

Already flagged by the Actions bot, still unfixed.

A question, not a change request: why is the thumbnail the part that is turned off?

The CmPicture Normal layout enables CmPicture-Detail-Picture (full size) and sets
CmPicture-Detail-Thumbnail to visibility="never" (LexSense.fwlayout:82-88).
PictureSlice.InstallPicture (PictureSlice.cs:59-73) does File.ReadAllBytes into a
MemoryStream into Image.FromStream with no size cap, per picture, per sense render on
the UI thread. Nobody has measured that on a picture-heavy lexicon because this code has
never run in this pane. Given a Thumbnail part was deliberately provided, flipping which
one is visible may be the cheaper default — but I would rather hear your reasoning than
assume mine.