Align navigated thread to content top

Thomas Randolphrequested to merge
tor/fix/discussion-nav-contenttop-alignment into master

What does this MR do and why?

Fixes flaky "Next/Previous open thread" navigation on the merge request Overview and Changes pages.

Root cause

The legacy navigation strategy in discussion_navigation.js decides which thread is "current" by measuring each thread's position relative to the contentTop() line (findNextClosestVisibleDiscussion computes y - contentTop(), and getPrevious then returns elements[index - 1]). That math is only correct if a just-navigated thread comes to rest exactly on the contentTop() line.

The scroll helper stopped guaranteeing that. scrollToDiscussion was changed to use scrollIntoView(true), which aligns the target to the top of the scroll container rather than to contentTop(). The navigated thread lands a few pixels above the line, so the next Next/Previous press miscounts the current thread by one: Previous returns the thread already in view, and Next leaves the expected thread covered by the sticky header. Because it depends on exact scroll position, it surfaces as a flaky spec rather than a hard failure.

This is a latent off-by-one exposed when the scroll mechanism was swapped from the old contentTop()-aligned scrollToElement to scrollIntoView + scrollPastCoveringElements in !228068 (merged). No single changed line looked wrong; the alignment invariant lived in a different function.

Fix

After scrolling the target into view, nudge the panel so the target's top sits on the contentTop() line again, restoring the invariant the navigation math relies on. scrollPastCoveringElements still runs afterward to clear any residual sticky coverage.

References

How to set up and validate locally

  1. Open a merge request that has two or more unresolved diff threads.
  2. On the Overview page, press n repeatedly to cycle forward through threads, then p to cycle back.
  3. Confirm each press lands on the adjacent thread (not the one already in view) and that the target thread is fully visible below the sticky header.

Merge request reports