LT-22489: Keep the selected browse row the same height as its neighbours

[jasonleenaylor]

Draw the selected row's highlight border at the same thickness as the
rule between rows instead of 3 points. The thicker border made the
selected row about three pixels taller, so moving the highlight to the
row above pushed the insertion point down and the SelectedIndex setter
then scrolled the view by that amount to put it back, which showed as a
small jump on every arrow-up.

Why it failed

XmlBrowseViewBaseVc.AddTableRow gave the selected row a 3000-millipoint top
border while every other row has the table's 750-millipoint rule. When the
index changes, the SelectedIndex setter snapshots the insertion point's
distance from the window top, re-lays out the old and new rows, and calls
RestoreSelectionAndScrollPos, which scrolls by the exact change in that
distance (the TE-6912 block). Moving up, the new row grows 3 px above the
insertion point, so the view scrolls 3 px. Moving down, the row above shrinks
by the same amount the new row grows, so nothing moves. Making the highlight
border the same thickness as the rule removes the height change; the peach
background remains, the blue top edge becomes 1 px.

Verification

• XMLViewsTests: 114 pass with -CommentHygiene -TokenHygiene.
• Reproduced Sara's LT-22489 recording on the restored Malayalam-Kunjumon backup
  with the pre-fix build: at the end of the list (Find > Go To "younger brother"),
  clicking another entry jumped the view up 34 px. On this branch the same clicks
  move nothing beyond the normal one-line spacing at the viewport edge.

Follow-up not in this PR

The jump Sara filmed has a second half that this branch only defuses, not fixes.
XmlBrowseViewBase.ScrollRange returns ScrollPositionMaxUserReachable + LargeChange - 1 (the WinForms ScrollBar.Maximum convention) while
SimpleRootSite.UpdateScrollRange treats that value as content height and
clamps the position to range - ClientHeight, which is one mean row short of the
real end. Anything that recomputes the scroll range while the view is within a
row of the end (pane/window resize, lazy-row expansion, an edit that changes a
row height) still pulls the view up by a row and hides the last entry (LT-13765
family). Removing the highlight-border height change means selection clicks no
longer trigger it. Fix candidate: override AdjustedScrollRange in
XmlBrowseViewBase to return ScrollPositionMaxUserReachable + ClientHeight.

Related: LT-8874 (2008, closed Won't Fix) is the earlier report of the same nudge.

Next: approve, or ask for the end-of-list clamp fix to ride along instead of
being a follow-up.

Preflight review details

Code Review Summary

Branch: LT-22489-selected-row-border-height
Base: origin/main
Date: 2026-09-23
Review model: Claude Fable 5.1 (Claude Code)
Files changed: 1

Overview

Purpose (author): arrow-up, and mouse clicks on a row above the current one,
nudge the browse view by about 3 px; in Lexicon Edit, Sara Mason's LT-22489
recording shows a full-row jump at the end of the list. The selected row's
highlight border was 3000 millipoints while every other row has the table's
750-millipoint rule, so selecting a row changed its height and the
SelectedIndex setter's RestoreSelectionAndScrollPos scrolled to compensate.
The branch draws the highlight border at the rule thickness through a shared
kdmpRowBorder constant in XmlBrowseViewBaseVc.AddTableRow, so selection no
longer changes row geometry.

Contract/API Changes

None. Visual change: the selected row's blue top edge is 1 px instead of 3 px;
the highlight background is unchanged.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

None.

Minor - Consider

• New constant's comment cited LT-20523 instead of LT-22489 (fixed
  during review: comment now cites LT-22489).
• The end-of-list clamp (XmlBrowseViewBase.ScrollRange returns the
  scrollbar-maximum form, SimpleRootSite.UpdateScrollRange clamps to
  range - ClientHeight, one mean row short of the end) is not fixed here.
  (author: "border only for now"; documented as a follow-up in the PR body.
  Verified on this branch that selection clicks no longer trigger it; a height
  resize at the end of the list still does.)

Required Validation / Evidence

• .\test.ps1 -SkipNative -TestProject XMLViewsTests -CommentHygiene -TokenHygiene
  in the worktree: 114 passed, hygiene clean (re-run after the comment fix).
• Reproduced LT-22489 on the restored Malayalam-Kunjumon backup with a
  pre-fix main build: at the end of the list after jumping to "younger
  brother", clicking another entry moved the view up 34 px
  (UpdateScrollRange clamp after the old row lost its 3 px border). On this
  branch the same clicks produce no movement beyond the normal one-line spacing
  at the viewport edge.
• Author manual check on this build: arrow up and down in Bulk Edit Phoneme
  Features and a lexicon with many entries, no nudge.

Positive Observations

• One constant replaces two literals, making the coupling between the rule and
  the highlight border explicit.

Interview Notes

• Author chose to keep the clamp fix out of this PR ("border only for now").
• Ticket: LT-22489 (open, FW 9.3.8). LT-8874 (2008, Won't Fix) is the earlier
  report of the same nudge. LT-20523/LT-22676 are not referenced.

In-Review Quality Check

Comment fix only; tests and hygiene re-run, see above.

Suggested Review Focus

• Accept the 1 px highlight edge as the new look for the selected row.

🤖 Generated with Claude Code

---

This change is 

[github-actions]

NUnit Tests

    1 files  ±0      1 suites  ±0   13m 3s ⏱️ + 2m 38s
6 232 tests ±0  6 147 ✅ ±0  85 💤 ±0  0 ❌ ±0 
6 241 runs  ±0  6 156 ✅ ±0  85 💤 ±0  0 ❌ ±0 

Results for commit e1b6ef0. ± Comparison against base commit 0ceeb22.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 38.95%. Comparing base (2f1d9db) to head (e1b6ef0).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1157      +/-   ##
==========================================
+ Coverage   38.89%   38.95%   +0.05%     
==========================================
  Files        1518     1520       +2     
  Lines      352262   352476     +214     
  Branches    40597    40636      +39     
==========================================
+ Hits       137005   137298     +293     
+ Misses     185976   185896      -80     
- Partials    29281    29282       +1     

Files with missing lines	Coverage Δ
...rc/Common/Controls/XMLViews/XmlBrowseViewBaseVc.cs	68.16% <100.00%> (ø)

... and 35 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.

[papeh]

LGTM; one question.

[papeh]

should this also be private const? Should we set kdmpRowBorder = m_dxmpCheckBorderWidth?

[jasonleenaylor]

Good question, I looked and then decided to keep them separate: m_dxmpCheckBorderWidth is the border drawn around the check-box picture (lines 791, 1324-1333) and kdmpRowBorder is the rule between rows. Both are 1 px today, but changing one should not move the other, and a const can't be initialised from an instance field anyway. m_dxmpCheckBorderWidth is never reassigned, so it could become const, but it is protected on a public class and unrelated to this fix, so I left it.