diff --git a/apps/readest-app/src/__tests__/document/paginator-table-toc-clip.browser.test.ts b/apps/readest-app/src/__tests__/document/paginator-table-toc-clip.browser.test.ts new file mode 100644 index 00000000..e5a8bf22 --- /dev/null +++ b/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((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('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); +}); diff --git a/apps/readest-app/src/__tests__/fixtures/data/repro-4439.epub b/apps/readest-app/src/__tests__/fixtures/data/repro-4439.epub new file mode 100644 index 00000000..8bbd4efa Binary files /dev/null and b/apps/readest-app/src/__tests__/fixtures/data/repro-4439.epub differ diff --git a/apps/readest-app/src/__tests__/utils/scrollable.test.ts b/apps/readest-app/src/__tests__/utils/scrollable.test.ts index 30f8664f..3a92ea3a 100644 --- a/apps/readest-app/src/__tests__/utils/scrollable.test.ts +++ b/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 = ` +
+
CONTENTS
+
+ `; + 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 = ` +
+
Cell
+
+ `; + 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 = `
diff --git a/apps/readest-app/src/utils/scrollable.ts b/apps/readest-app/src/utils/scrollable.ts index 010a445a..181c16be 100644 --- a/apps/readest-app/src/utils/scrollable.ts +++ b/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.