Correctly handle overflowing content when choosing scroll anchor

Fix two issues found while testing HTML side-by-side mode with a BBC
news article [1]:

1. When traversing the DOM tree to choose a scroll anchor, elements were
   skipped if their bounding rect (`element.getBoundingClientRect()`) did
   not intersect the viewport. This however did not account for content
   which overflows the element but is still visible to the user because it
   is not clipped.

2. The code used the intersection of the scroll root's bounding client rect and
   `(0, 0, window.innerWidth, window.innerHeight)` to get the viewport
   rect. In [1] the root element's bounding rect only covers the top
   (100vw, 100vh) pixels of the document, and most of the actual content
   of the page overflows that area. Resolve the issue by always using
   `(0, 0, window.innerWidth, window.innerHeight)` as the viewport,
   except when explicitly overridden (eg. in tests)

[1] https://www.bbc.co.uk/news/world-europe-60693166

src/annotator/integrations/html-side-by-side.js

-import { intersectRects, rectContains, rectIntersects } from '../util/geometry';
+import { rectContains, rectIntersects } from '../util/geometry';
 
 /**
  * CSS selectors used to find elements that are considered potentially part
@@ -108,6 +108,21 @@ function hasFixedPosition(element) {
   }
 }
 
+/**
+ * Return the bounding rect that contains the element's content, including
+ * any content which overflows the element's specified size.
+ *
+ * @param {Element} element
+ */
+function elementContentRect(element) {
+  const rect = element.getBoundingClientRect();
+  rect.x -= element.scrollLeft;
+  rect.y -= element.scrollTop;
+  rect.height = Math.max(rect.height, element.scrollHeight);
+  rect.width = Math.max(rect.width, element.scrollWidth);
+  return rect;
+}
+
 /**
  * Yield all the text node descendants of `root` that intersect `rect`.
  *
@@ -123,10 +138,13 @@ function* textNodesInRect(root, rect, shouldVisit = () => true) {
   while (node) {
     if (node.nodeType === Node.ELEMENT_NODE) {
       const element = /** @type {Element} */ (node);
-      const elementRect = element.getBoundingClientRect();
+      const contentIntersectsRect = rectIntersects(
+        elementContentRect(element),
+        rect
+      );
 
-      // Only examine subtrees which are visible and intersect the viewport.
-      if (shouldVisit(element) && rectIntersects(elementRect, rect)) {
+      // Only examine subtrees which are visible.
+      if (shouldVisit(element) && contentIntersectsRect) {
         yield* textNodesInRect(element, rect, shouldVisit);
       }
     } else if (node.nodeType === Node.TEXT_NODE) {
@@ -145,24 +163,16 @@ function* textNodesInRect(root, rect, shouldVisit = () => true) {
  * Find content within an element to use as an anchor when applying a layout
  * change to the document.
  *
- * @param {Element} scrollRoot
+ * @param {Element} root
+ * @param {DOMRect} viewport
  * @return {Range|null} - Range to use as an anchor or `null` if a suitable
  *   range could not be found
  */
-function getScrollAnchor(scrollRoot) {
+function getScrollAnchor(root, viewport) {
   // Range representing the content whose position within the viewport we will
   // try to maintain after running the callback.
   let anchorRange = /** @type {Range|null} */ (null);
 
-  const viewport = intersectRects(
-    scrollRoot.getBoundingClientRect(),
-    new DOMRect(0, 0, window.innerWidth, window.innerHeight)
-  );
-  if (viewport.width < 0 || viewport.height < 0) {
-    // Element being scrolled is outside the viewport
-    return null;
-  }
-
   // Find the first word (non-whitespace substring of a text node) that is fully
   // visible in the viewport.
 
@@ -172,7 +182,7 @@ function getScrollAnchor(scrollRoot) {
   const shouldVisit = el => !hasFixedPosition(el);
 
   textNodeLoop: for (let textNode of textNodesInRect(
-    scrollRoot,
+    root,
     viewport,
     shouldVisit
   )) {
@@ -208,15 +218,19 @@ function getScrollAnchor(scrollRoot) {
  *
  * @param {() => any} callback - Callback that will apply the layout change
  * @param {Element} [scrollRoot]
+ * @param {DOMRect} [viewport] - Area to consider "in the viewport". Defaults to
+ *   the viewport of the current window.
  * @return {number} - Amount by which the scroll position was adjusted to keep
  *   the anchored content in view
  */
 export function preserveScrollPosition(
   callback,
   /* istanbul ignore next */
-  scrollRoot = document.documentElement
+  scrollRoot = document.documentElement,
+  /* istanbul ignore next */
+  viewport = new DOMRect(0, 0, window.innerWidth, window.innerHeight)
 ) {
-  const anchor = getScrollAnchor(scrollRoot);
+  const anchor = getScrollAnchor(scrollRoot, viewport);
   if (!anchor) {
     callback();
     return 0;

src/annotator/integrations/test/html-side-by-side-test.js

@@ -142,16 +142,19 @@ the fighting was.`;
       scrollRoot.remove();
     });
 
-    it('selects content as a scroll anchor and preserves its position in the viewport', () => {
-      scrollRoot.scrollTop = 200;
+    function resizeViewportAndExpectScrollAdjustment() {
       const initialScrollTop = scrollRoot.scrollTop;
 
-      const delta = preserveScrollPosition(() => {
+      const delta = preserveScrollPosition(
+        () => {
           // Make the viewport narrower. This will make the content taller and
           // require `preserveScrollPosition` to adjust the scroll offset to keep
           // the anchored content visible.
           scrollRoot.style.width = '150px';
-      }, scrollRoot);
+        },
+        scrollRoot,
+        scrollRoot.getBoundingClientRect()
+      );
 
       assert.notEqual(delta, 0);
 
@@ -161,6 +164,19 @@ the fighting was.`;
         Math.floor(scrollRoot.scrollTop),
         Math.floor(initialScrollTop + delta)
       );
+    }
+
+    it('selects content as a scroll anchor and preserves its position in the viewport', () => {
+      scrollRoot.scrollTop = 200;
+      resizeViewportAndExpectScrollAdjustment();
+    });
+
+    it('selects scroll anchor if it is part of overflowing content', () => {
+      // Give the content a small height so that the text in the viewport is
+      // part of overflowing content.
+      content.style.height = '10px';
+      scrollRoot.scrollTop = 200;
+      resizeViewportAndExpectScrollAdjustment();
     });
 
     it('ignores fixed-position content when choosing a scroll anchor', () => {
@@ -188,13 +204,10 @@ the fighting was.`;
       scrollRoot.append(inner);
 
       scrollRoot.scrollTop = 200;
-      const delta = preserveScrollPosition(() => {
-        scrollRoot.style.width = '150px';
-      }, scrollRoot);
 
-      // The scroll position should be adjusted. This would be zero if the
-      // text in the <nav> element was used as a scroll anchor.
-      assert.notEqual(delta, 0);
+      // Resize viewport and check scroll position is adjusted. It would not be
+      // adjusted if the text in the <nav> element was used as a scroll anchor.
+      resizeViewportAndExpectScrollAdjustment();
     });
 
     it('does not restore the scroll position if no anchor content could be found', () => {
@@ -203,30 +216,39 @@ the fighting was.`;
       scrollRoot.scrollTop = 200;
       const initialScrollTop = scrollRoot.scrollTop;
 
-      const delta = preserveScrollPosition(() => {
+      const delta = preserveScrollPosition(
+        () => {
           // Make the viewport narrower. This will make the content taller and
           // require `preserveScrollPosition` to adjust the scroll offset to keep
           // the anchored content visible.
           scrollRoot.style.width = '150px';
-      }, scrollRoot);
+        },
+        scrollRoot,
+        scrollRoot.getBoundingClientRect()
+      );
 
       assert.equal(delta, 0);
       assert.equal(scrollRoot.scrollTop, initialScrollTop);
     });
 
-    it('does nothing if scroll element is outside viewport', () => {
+    it('does nothing if scroll root is outside viewport', () => {
       scrollRoot.style.position = 'absolute';
       scrollRoot.style.top = '10000px';
 
       scrollRoot.scrollTop = 200;
       const initialScrollTop = scrollRoot.scrollTop;
 
-      const delta = preserveScrollPosition(() => {
+      const delta = preserveScrollPosition(
+        () => {
           // Make the viewport narrower. This will make the content taller and
           // require `preserveScrollPosition` to adjust the scroll offset to keep
           // the anchored content visible.
           scrollRoot.style.width = '150px';
-      }, scrollRoot);
+        },
+        scrollRoot,
+        // Viewport
+        new DOMRect(0, 0, 800, 600)
+      );
 
       assert.equal(delta, 0);
       assert.equal(scrollRoot.scrollTop, initialScrollTop);