Find Mode: Fix scrolling to matches in scrollable containers (e.g., GitLab)

[lucas6028]

Problem

I was trying to fix issue #4827, which is about finding mode fails to scroll to search results on websites that use scrollable container elements instead of window scrolling (e.g., GitLab). The matched text is found and highlighted correctly, but the viewport doesn't scroll to reveal it.

Root cause: The original implementation used globalThis.scrollTo() which only scrolls the main window. On sites like GitLab, content is inside a nested scrollable <div>, so window.scrollY stays at 0.

Proposed Solutions
Option A: Use scrollIntoView() (Simpler)

const highlight = (textNode, startIndex, length) => {
  // ... selection setup ...
  
  // Use scrollIntoView to properly handle scrollable containers.
  const span = document.createElement("span");
  range.insertNode(span);
  span.scrollIntoView({
    behavior: "instant",
    block: "center",
  });
  span.remove();
  // Restore selection after DOM manipulation
  selection.removeAllRanges();
  selection.addRange(range);
  return true;
};

Pros:

• Simple, minimal code
• Browser handles all scrollable ancestor detection automatically
• Works for deeply nested scrollable containers

Cons:

• Requires temporary DOM insertion/removal
• Always scrolls (even if element is already visible) — though scrollIntoView is optimized
• Less explicit control over scroll behavior

Option B: Manual scrollable parent detection (More explicit)

const getScrollableParent = (element) => {
  let node = element;
  while (
    node && (node instanceof Element) && (node !== document.body) &&
    (node !== document.documentElement)
  ) {
    const style = globalThis.getComputedStyle(node);
    const overflowY = style.overflowY;
    if (
      ((overflowY === "auto") || (overflowY === "scroll")) &&
      (node.scrollHeight > node.clientHeight)
    ) {
      return node;
    }
    node = node.parentElement;
  }
  return null;
};
const highlight = (textNode, startIndex, length) => {
  // ... selection setup ...
  let rect = range.getBoundingClientRect();
  const scrollParent = getScrollableParent(textNode.parentElement);
  if (scrollParent) {
    const parentRect = scrollParent.getBoundingClientRect();
    if ((rect.top < parentRect.top) || (rect.bottom > parentRect.bottom)) {
      const offset = (rect.top - parentRect.top) + (rect.height / 2) -
        (scrollParent.clientHeight / 2);
      scrollParent.scrollBy({ top: offset, behavior: "instant" });
      rect = range.getBoundingClientRect();
    }
  }
  // Fallback to window scroll
  if (rect.top < 0 || rect.bottom > globalThis.innerHeight) {
    globalThis.scrollTo({
      top: globalThis.scrollY + rect.top + rect.height / 2 - screenHeight / 2,
      behavior: "instant",
    });
  }
  return true;
};

Pros:

• No DOM manipulation required
• Explicit visibility checks (only scrolls when necessary)
• Handles both container scroll AND window scroll as fallback
• More predictable behavior

Cons:

• More code to maintain
• Only finds the first scrollable ancestor (may miss deeply nested cases)
• Manual visibility calculation could have edge cases

---

Questions for Discussion

1. Is the simplicity of scrollIntoView() worth the temporary DOM insertion?
2. Are there edge cases where scrollIntoView() behaves unexpectedly?
3. Does Option B need to recursively scroll multiple ancestors?
4. Is there any other solution for this issue?