From 4d0be496b298c05bb0624c612664e8cdff1fd9d2 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Thu, 2 Jul 2026 00:19:20 +0900 Subject: [PATCH] fix(layout): respect author vertical-align on inline images (#4866) (#4878) img.has-text-siblings forced vertical-align: baseline on every inline image with text siblings, out-specifying a book's own value (for example a CJK glyph-substitution image nudged with vertical-align: -0.15em). Because baseline is the CSS initial value, the declaration only ever mattered when it clobbered an authored value. Keep baseline as a default only: move it to a new has-text-siblings-baseline class that applyImageStyle adds only when the image has no author-set vertical-align (detected via getComputedStyle). Refactor applyImageStyle to a two-phase read-then-write pass to avoid a getComputedStyle-after-write style recalc. Co-authored-by: Claude Opus 4.8 (1M context) --- .../src/__tests__/utils/style-dom.test.ts | 44 ++++++++++ apps/readest-app/src/utils/style.ts | 81 +++++++++++++------ 2 files changed, 99 insertions(+), 26 deletions(-) diff --git a/apps/readest-app/src/__tests__/utils/style-dom.test.ts b/apps/readest-app/src/__tests__/utils/style-dom.test.ts index 81986365..3c132d2c 100644 --- a/apps/readest-app/src/__tests__/utils/style-dom.test.ts +++ b/apps/readest-app/src/__tests__/utils/style-dom.test.ts @@ -417,6 +417,33 @@ describe('getStyles', () => { expect(css).toContain('background-color: #ffffff !important'); expect(css).toContain('color: #171717 !important'); }); + + it('scopes the inline-image baseline default to its own class so author values win #4866', () => { + const vs = makeViewSettings(); + const themeCode: ThemeCode = { + bg: '#ffffff', + fg: '#171717', + primary: '#0066cc', + palette: { + 'base-100': '#ffffff', + 'base-200': '#f2f2f2', + 'base-300': '#e0e0e0', + 'base-content': '#171717', + neutral: '#cccccc', + 'neutral-content': '#444444', + primary: '#0066cc', + secondary: '#3399ff', + accent: '#0055aa', + }, + isDarkMode: false, + }; + const css = getStyles(vs, themeCode); + // Baseline is opt-in via a dedicated class, not forced on every inline image. + expect(css).toMatch(/img\.has-text-siblings-baseline\s*\{[^}]*vertical-align:\s*baseline/); + // The size-clamp rule must not carry vertical-align anymore. + const clampRule = css.match(/\n\s*img\.has-text-siblings\s*\{[^}]*\}/)![0]; + expect(clampRule).not.toContain('vertical-align'); + }); }); // --------------------------------------------------------------------------- @@ -479,6 +506,23 @@ describe('applyImageStyle', () => { const img = document.querySelector('img')!; expect(img.classList.contains('has-text-siblings')).toBe(false); }); + + it('applies the baseline default when the image has no author vertical-align', () => { + document.body.innerHTML = '

Some text more text

'; + applyImageStyle(document); + const img = document.querySelector('img')!; + expect(img.classList.contains('has-text-siblings')).toBe(true); + expect(img.classList.contains('has-text-siblings-baseline')).toBe(true); + }); + + it('respects an author-set vertical-align (no baseline default) #4866', () => { + document.body.innerHTML = + '

Some text more text

'; + applyImageStyle(document); + const img = document.querySelector('img')!; + expect(img.classList.contains('has-text-siblings')).toBe(true); + expect(img.classList.contains('has-text-siblings-baseline')).toBe(false); + }); }); // --------------------------------------------------------------------------- diff --git a/apps/readest-app/src/utils/style.ts b/apps/readest-app/src/utils/style.ts index 5ff089ed..3e251d09 100644 --- a/apps/readest-app/src/utils/style.ts +++ b/apps/readest-app/src/utils/style.ts @@ -472,6 +472,12 @@ const getPageLayoutStyles = ( } img.has-text-siblings { ${vertical ? 'width: 1em;' : 'height: 1em;'} + } + /* Baseline is only Readest's default: applyImageStyle adds this class when the + book leaves vertical-align at its initial value, so an author-set value + (e.g. a CJK glyph-substitution image nudged with vertical-align: -0.15em) + keeps winning. See #4866. */ + img.has-text-siblings-baseline { vertical-align: baseline; } :is(div) > img.has-text-siblings[style*="object-fit"] { @@ -1148,37 +1154,60 @@ export const applyScrollbarStyle = (document: Document, hideScrollbar: boolean) }; export const applyImageStyle = (document: Document) => { - document.querySelectorAll('img').forEach((img) => { + const win = document.defaultView ?? window; + // Two-phase (read then write): reading getComputedStyle after a class/style + // write forces a style recalc, so gather every decision first and apply the + // mutations afterwards (same reasoning as keepTextAlignment's split). + const plans = Array.from(document.querySelectorAll('img')).map((img) => { const widthAttr = img.getAttribute('width'); - if (widthAttr && (widthAttr.endsWith('%') || widthAttr.endsWith('vw'))) { - const percentage = parseFloat(widthAttr); - if (!isNaN(percentage)) { - img.style.width = `${(percentage / 100) * window.innerWidth}px`; - img.removeAttribute('width'); - } - } - + const percentWidth = + widthAttr && (widthAttr.endsWith('%') || widthAttr.endsWith('vw')) + ? parseFloat(widthAttr) + : NaN; const heightAttr = img.getAttribute('height'); - if (heightAttr && (heightAttr.endsWith('%') || heightAttr.endsWith('vh'))) { - const percentage = parseFloat(heightAttr); - if (!isNaN(percentage)) { - img.style.height = `${(percentage / 100) * window.innerHeight}px`; - img.removeAttribute('height'); + const percentHeight = + heightAttr && (heightAttr.endsWith('%') || heightAttr.endsWith('vh')) + ? parseFloat(heightAttr) + : NaN; + + let inlineWithText = false; + let keepBaseline = false; + const parent = img.parentNode; + if (parent && parent.nodeType === Node.ELEMENT_NODE) { + const childNodes = Array.from(parent.childNodes); + const hasTextSiblings = childNodes.some( + (node) => node.nodeType === Node.TEXT_NODE && node.textContent?.trim(), + ); + const isInline = childNodes.every( + (node) => node.nodeType !== Node.ELEMENT_NODE || (node as Element).tagName !== 'BR', + ); + inlineWithText = hasTextSiblings && isInline; + if (inlineWithText) { + // Only supply Readest's baseline default when the book leaves + // vertical-align at its initial value; an author-set value (e.g. a CJK + // glyph-substitution image nudged with `vertical-align: -0.15em`) must + // win. Empty string covers environments that report unset props as ''. + const valign = win.getComputedStyle(img).verticalAlign; + keepBaseline = valign === '' || valign === 'baseline'; } } - - const parent = img.parentNode; - if (!parent || parent.nodeType !== Node.ELEMENT_NODE) return; - const hasTextSiblings = Array.from(parent.childNodes).some( - (node) => node.nodeType === Node.TEXT_NODE && node.textContent?.trim(), - ); - const isInline = Array.from(parent.childNodes).every( - (node) => node.nodeType !== Node.ELEMENT_NODE || (node as Element).tagName !== 'BR', - ); - if (hasTextSiblings && isInline) { - img.classList.add('has-text-siblings'); - } + return { img, percentWidth, percentHeight, inlineWithText, keepBaseline }; }); + + for (const { img, percentWidth, percentHeight, inlineWithText, keepBaseline } of plans) { + if (!isNaN(percentWidth)) { + img.style.width = `${(percentWidth / 100) * window.innerWidth}px`; + img.removeAttribute('width'); + } + if (!isNaN(percentHeight)) { + img.style.height = `${(percentHeight / 100) * window.innerHeight}px`; + img.removeAttribute('height'); + } + if (inlineWithText) { + img.classList.add('has-text-siblings'); + if (keepBaseline) img.classList.add('has-text-siblings-baseline'); + } + } document.querySelectorAll('hr').forEach((hr) => { const computedStyle = window.getComputedStyle(hr); if (computedStyle.backgroundImage && computedStyle.backgroundImage !== 'none') {