fix(reader): keep negative table margins from clipping wrapped layout tables (#4439) (#4808)

A decorative table-of-contents page lays out as nested tables where the inner
table pulls itself up with a negative top margin and the CONTENTS heading uses
line-height:1em. Since #4400 wraps every table in a `.scroll-wrapper`
(overflow:auto), that negative margin bled the heading above the wrapper's clip
box and overflow cut off the top half of its glyphs.

Hoist any negative margins from the wrapped element onto the wrapper and zero
them on the element: the box stays in place, the element sits flush inside it so
overflow cannot clip it, and scrollWidth is no longer inflated by the margin so a
table that actually fits still gets marked fit. Positive and auto margins are
left alone, so an over-wide table still scrolls and a centered table stays
centered.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

apps/readest-app/src/__tests__/document/paginator-table-toc-clip.browser.test.ts

@@ -0,0 +1,142 @@
+import { describe, it, expect, beforeAll, afterEach } from 'vitest';
+import { DocumentLoader } from '@/libs/document';
+import type { BookDoc } from '@/libs/document';
+import type { ViewSettings } from '@/types/book';
+import type { Renderer } from '@/types/view';
+import { getStyles } from '@/utils/style';
+import { applyScrollableStyle, SCROLL_WRAPPER_CLASS } from '@/utils/scrollable';
+import {
+  DEFAULT_BOOK_FONT,
+  DEFAULT_BOOK_LAYOUT,
+  DEFAULT_BOOK_STYLE,
+  DEFAULT_BOOK_LANGUAGE,
+  DEFAULT_VIEW_CONFIG,
+  DEFAULT_TTS_CONFIG,
+  DEFAULT_TRANSLATOR_CONFIG,
+  DEFAULT_ANNOTATOR_CONFIG,
+  DEFAULT_SCREEN_CONFIG,
+} from '@/services/constants';
+
+// repro-4439.epub: a decorative table-of-contents page laid out as nested
+// tables. The inner contents table has a negative top margin and the CONTENTS
+// heading uses line-height:1em. Wrapping the table in a `.scroll-wrapper`
+// (#4400) turned it into an overflow:auto clip box, and the negative margin bled
+// the top of the heading above that box, cutting the glyphs in half (#4439).
+const TOC_EPUB_URL = new URL('../fixtures/data/repro-4439.epub', import.meta.url).href;
+// The contents page is the second spine item (cover, contents, ch1).
+const TOC_SECTION_INDEX = 1;
+const TOLERANCE_PX = 2;
+
+const loadEPUB = async (url: string, name: string) => {
+  const resp = await fetch(url);
+  const buffer = await resp.arrayBuffer();
+  const file = new File([buffer], name, { type: 'application/epub+zip' });
+  const { book } = await new DocumentLoader(file).open();
+  return book;
+};
+
+const makeViewSettings = (): ViewSettings =>
+  ({
+    ...DEFAULT_BOOK_FONT,
+    ...DEFAULT_BOOK_LAYOUT,
+    ...DEFAULT_BOOK_STYLE,
+    ...DEFAULT_BOOK_LANGUAGE,
+    ...DEFAULT_VIEW_CONFIG,
+    ...DEFAULT_TTS_CONFIG,
+    ...DEFAULT_TRANSLATOR_CONFIG,
+    ...DEFAULT_ANNOTATOR_CONFIG,
+    ...DEFAULT_SCREEN_CONFIG,
+  }) as ViewSettings;
+
+const waitForStabilized = (el: HTMLElement, timeout = 10000) =>
+  new Promise<void>((resolve, reject) => {
+    const timer = setTimeout(() => reject(new Error('stabilized timeout')), timeout);
+    el.addEventListener(
+      'stabilized',
+      () => {
+        clearTimeout(timer);
+        resolve();
+      },
+      { once: true },
+    );
+  });
+
+const getSectionDoc = (paginator: Renderer, index: number): Document | null =>
+  paginator.getContents().find((c) => c.index === index)?.doc ?? null;
+
+// Nearest ancestor that clips its overflow (the scroll-wrapper, when scrollable).
+const nearestClipAncestor = (el: HTMLElement): HTMLElement | null => {
+  let cur: HTMLElement | null = el.parentElement;
+  const win = el.ownerDocument.defaultView!;
+  while (cur) {
+    if (win.getComputedStyle(cur).overflow !== 'visible') return cur;
+    cur = cur.parentElement;
+  }
+  return null;
+};
+
+let tocBook: BookDoc;
+
+describe('Paginator decorative TOC table', () => {
+  let paginator: Renderer;
+
+  beforeAll(async () => {
+    tocBook = await loadEPUB(TOC_EPUB_URL, 'repro-4439.epub');
+    await import('foliate-js/paginator.js');
+  }, 30000);
+
+  afterEach(() => {
+    if (paginator) {
+      try {
+        paginator.destroy();
+      } catch {
+        /* iframe body may already be torn down */
+      }
+      paginator.remove();
+    }
+  });
+
+  it('does not clip the top of a heading inside a wrapped layout table', async () => {
+    const el = document.createElement('foliate-paginator') as Renderer;
+    Object.assign(el.style, {
+      width: '320px',
+      height: '800px',
+      position: 'absolute',
+      left: '0',
+      top: '0',
+    });
+    document.body.appendChild(el);
+    paginator = el;
+
+    paginator.open(tocBook);
+    paginator.setStyles?.(getStyles(makeViewSettings(), undefined, []));
+
+    const stabilized = waitForStabilized(paginator);
+    await paginator.goTo({ index: TOC_SECTION_INDEX });
+    await stabilized;
+
+    const doc = getSectionDoc(paginator, TOC_SECTION_INDEX);
+    expect(doc, 'contents section doc').toBeTruthy();
+    paginator.setStyles?.(getStyles(makeViewSettings(), undefined, []));
+    applyScrollableStyle(doc!);
+
+    const heading = [...doc!.querySelectorAll<HTMLElement>('p.title')].find((p) =>
+      /CONTENTS/.test(p.textContent ?? ''),
+    );
+    expect(heading, 'CONTENTS heading').toBeTruthy();
+
+    // It must sit inside a scroll-wrapper (otherwise the test would not be
+    // exercising the clip path that #4400 introduced).
+    expect(heading!.closest(`.${SCROLL_WRAPPER_CLASS}`), 'heading is wrapped').toBeTruthy();
+
+    const clip = nearestClipAncestor(heading!);
+    if (clip) {
+      const headTop = heading!.getBoundingClientRect().top;
+      const clipTop = clip.getBoundingClientRect().top;
+      expect(
+        headTop,
+        `CONTENTS top (${headTop.toFixed(1)}) is clipped above its overflow box top (${clipTop.toFixed(1)})`,
+      ).toBeGreaterThanOrEqual(clipTop - TOLERANCE_PX);
+    }
+  }, 60000);
+});

apps/readest-app/src/__tests__/utils/scrollable.test.ts

@@ -67,6 +67,42 @@ describe('applyScrollableStyle', () => {
     expect(table.parentElement?.classList.contains(SCROLL_WRAPPER_CLASS)).toBe(true);
   });
 
+  it('hoists a negative table margin onto the wrapper so it is not clipped (#4439)', () => {
+    // A decorative TOC lays out as nested tables; the inner table pulls itself
+    // up with a negative top margin. Inside an overflow:auto wrapper that margin
+    // would bleed the table (and its heading) past the box edge and get clipped.
+    document.body.innerHTML = `
+      <div>
+        <table style="margin: -1em 0 0 1em;"><tr><td>CONTENTS</td></tr></table>
+      </div>
+    `;
+    applyScrollableStyle(document);
+    const table = document.querySelector('table')!;
+    const wrapper = table.parentElement!;
+    expect(wrapper.classList.contains(SCROLL_WRAPPER_CLASS)).toBe(true);
+    // The negative top margin moves to the wrapper (box stays in place)...
+    expect(wrapper.style.marginTop).toBe('-1em');
+    // ...and is zeroed on the table so it sits flush inside the clip box.
+    expect(table.style.marginTop).toBe('0px');
+    // The positive left margin is left alone (still counts toward overflow).
+    expect(wrapper.style.marginLeft).toBe('');
+    expect(table.style.marginLeft).toBe('1em');
+  });
+
+  it('leaves a table with only non-negative margins untouched', () => {
+    document.body.innerHTML = `
+      <div>
+        <table style="margin: 1em auto;"><tr><td>Cell</td></tr></table>
+      </div>
+    `;
+    applyScrollableStyle(document);
+    const table = document.querySelector('table')!;
+    const wrapper = table.parentElement!;
+    expect(wrapper.style.marginTop).toBe('');
+    expect(table.style.marginTop).toBe('1em');
+    expect(table.style.marginLeft).toBe('auto');
+  });
+
   it('wraps a display equation (math that is the sole content of its container)', () => {
     document.body.innerHTML = `
       <div data-type="equation">

apps/readest-app/src/utils/scrollable.ts

@@ -169,6 +169,7 @@ export const applyScrollableStyle = (document: Document) => {
     wrapper.setAttribute('cfi-skip', '');
     parent.insertBefore(wrapper, el);
     wrapper.appendChild(el);
+    hoistNegativeMargins(el, wrapper, win);
     decideTableFit(wrapper, win);
   };
   document.querySelectorAll('table').forEach(wrap);
@@ -177,6 +178,36 @@ export const applyScrollableStyle = (document: Document) => {
   });
 };
 
+/**
+ * Move any NEGATIVE margins from the wrapped element onto the wrapper, zeroing
+ * them on the element. The wrapper is an overflow:auto clip box; a negative
+ * margin on the element pulls its content past the box edge, where overflow
+ * clips it — a decorative TOC laid out with nested tables (`margin: -1em`) had
+ * the top half of its heading cut off this way (#4439). Hoisting keeps the box
+ * in the same place while letting the element sit flush inside it. Positive and
+ * auto margins are left alone, so a genuinely over-wide table still counts as
+ * overflowing and a centered table stays centered.
+ */
+const MARGIN_SIDES = ['Top', 'Right', 'Bottom', 'Left'] as const;
+const hoistNegativeMargins = (
+  el: Element,
+  wrapper: HTMLElement,
+  win: (Window & typeof globalThis) | null,
+) => {
+  if (!win) return;
+  const style = (el as HTMLElement).style;
+  if (!style) return;
+  const computed = win.getComputedStyle(el);
+  for (const side of MARGIN_SIDES) {
+    const prop = `margin${side}` as 'marginTop';
+    const value = computed[prop];
+    if (parseFloat(value) < 0) {
+      wrapper.style[prop] = value;
+      style[prop] = '0px';
+    }
+  }
+};
+
 /**
  * Toggle SCROLL_WRAPPER_FIT_CLASS: a table within tolerance of its column fits
  * (overflow:visible — not a scroll container), a wider one stays overflow:auto.