From 49b171f5e505bd5e30d9609b0f76806b8fe2840e Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Sat, 23 May 2026 23:17:04 +0800 Subject: [PATCH] fix(reader): restore right-column clicks and selection in dual-page mode (#4283) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(reader): restore right-column clicks and selection in dual-page mode Bumps foliate-js to readest/foliate-js@1ea2996, which stops marking visible non-primary views as `inert` / `aria-hidden`. Adds a regression test for the underlying visibility helper. When a dual-page spread crosses a section boundary, the right column lives in a non-primary view. The previous a11y sync set both `inert` and `aria-hidden` on that view — `inert` blocked link clicks and text selection in the visible column, and `aria-hidden` hid it from assistive tech while sighted users could still read it. The fix drops `inert` entirely (screen-reader swipe-next is already handled by `aria-hidden`) and applies `aria-hidden` only to views whose bounding rect lies outside the visible container. Fixes #4243 (right-column links unclickable in dual-page mode). Fixes #4259 (text selection fails when an image section is on the left). Co-Authored-By: Claude Opus 4.7 (1M context) * test(reader): update paginator-multiview a11y assertions for new visibility-based aria-hidden Browser tests previously assumed every non-primary view carried both `inert` and `aria-hidden`. The fix in the preceding commit drops `inert` entirely and only `aria-hidden`s views that are off-screen. Update the two assertions to: - check that no wrapper ever has `inert`; - require `aria-hidden="true"` only when the wrapper's bounding rect is fully outside the visible container, and require its absence when the wrapper overlaps the viewport (the regression case from #4243 / #4259). Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../paginator-multiview.browser.test.ts | 42 ++++++++++-- .../__tests__/foliate-paginator-a11y.test.ts | 65 +++++++++++++++++++ packages/foliate-js | 2 +- 3 files changed, 101 insertions(+), 8 deletions(-) create mode 100644 apps/readest-app/src/__tests__/foliate-paginator-a11y.test.ts diff --git a/apps/readest-app/src/__tests__/document/paginator-multiview.browser.test.ts b/apps/readest-app/src/__tests__/document/paginator-multiview.browser.test.ts index 371cb6da..695c3028 100644 --- a/apps/readest-app/src/__tests__/document/paginator-multiview.browser.test.ts +++ b/apps/readest-app/src/__tests__/document/paginator-multiview.browser.test.ts @@ -200,15 +200,27 @@ describe('Paginator multi-view architecture (browser)', () => { }); }); - describe('Accessibility: non-primary views are hidden from AT', () => { + describe('Accessibility: off-screen pre-loaded views are hidden from AT', () => { const getViewWrappers = (el: Renderer) => { const container = el.shadowRoot?.getElementById('container'); return container ? (Array.from(container.children) as HTMLElement[]) : []; }; const wrapperContainsIframeForIndex = (wrapper: HTMLElement, doc: Document) => wrapper.querySelector('iframe')?.contentDocument === doc; + // Visible non-primary views (e.g. the right column in a dual-page spread + // belonging to a different section than the left column) must stay + // interactive and exposed to assistive tech. Only views whose bounding + // rect lies fully outside the visible container area are aria-hidden. + // See readest/readest#4243 and #4259. + const isOffScreen = (wrapper: HTMLElement, el: Renderer) => { + const container = el.shadowRoot?.getElementById('container'); + if (!container) return false; + const c = container.getBoundingClientRect(); + const w = wrapper.getBoundingClientRect(); + return !(w.right > c.left && w.left < c.right && w.bottom > c.top && w.top < c.bottom); + }; - it('should mark non-primary view wrappers inert + aria-hidden', async () => { + it('keeps the primary view interactive and never sets inert', async () => { const firstLinear = book.sections!.findIndex((s) => s.linear !== 'no'); await setupAt(firstLinear); await waitForViews(paginator, 2); @@ -226,15 +238,24 @@ describe('Paginator multi-view architecture (browser)', () => { expect(primaryWrapper!.hasAttribute('inert')).toBe(false); expect(primaryWrapper!.getAttribute('aria-hidden')).not.toBe('true'); + for (const w of wrappers) { + // No wrapper should ever carry `inert` — it would block link clicks + // and selection in any visible column. + expect(w.hasAttribute('inert')).toBe(false); + } + for (const np of nonPrimary) { const wrapper = wrappers.find((w) => wrapperContainsIframeForIndex(w, np.doc)); expect(wrapper).toBeDefined(); - expect(wrapper!.hasAttribute('inert')).toBe(true); - expect(wrapper!.getAttribute('aria-hidden')).toBe('true'); + if (isOffScreen(wrapper!, paginator)) { + expect(wrapper!.getAttribute('aria-hidden')).toBe('true'); + } else { + expect(wrapper!.getAttribute('aria-hidden')).not.toBe('true'); + } } }); - it('should move inert + aria-hidden when the primary section changes', async () => { + it('updates aria-hidden when the primary section changes', async () => { const linearSections = book .sections!.map((s, i) => ({ s, i })) .filter(({ s }) => s.linear !== 'no'); @@ -259,12 +280,19 @@ describe('Paginator multi-view architecture (browser)', () => { expect(primaryWrapper!.hasAttribute('inert')).toBe(false); expect(primaryWrapper!.getAttribute('aria-hidden')).not.toBe('true'); + for (const w of wrappers) { + expect(w.hasAttribute('inert')).toBe(false); + } + const nonPrimary = contents.filter((c) => c.index !== second); for (const np of nonPrimary) { const wrapper = wrappers.find((w) => wrapperContainsIframeForIndex(w, np.doc)); expect(wrapper).toBeDefined(); - expect(wrapper!.hasAttribute('inert')).toBe(true); - expect(wrapper!.getAttribute('aria-hidden')).toBe('true'); + if (isOffScreen(wrapper!, paginator)) { + expect(wrapper!.getAttribute('aria-hidden')).toBe('true'); + } else { + expect(wrapper!.getAttribute('aria-hidden')).not.toBe('true'); + } } }); }); diff --git a/apps/readest-app/src/__tests__/foliate-paginator-a11y.test.ts b/apps/readest-app/src/__tests__/foliate-paginator-a11y.test.ts new file mode 100644 index 00000000..e5934902 --- /dev/null +++ b/apps/readest-app/src/__tests__/foliate-paginator-a11y.test.ts @@ -0,0 +1,65 @@ +// Regression test for readest/readest issues #4243 and #4259. +// +// In dual-page mode, when adjacent columns belong to *different* sections, +// the section in the right column lives in a non-primary view. Such visible +// non-primary views must not be marked `aria-hidden` (and must not be made +// `inert`), otherwise screen readers skip them and link clicks plus text +// selection break in the right column. +import { describe, expect, it } from 'vitest'; + +import { isViewVisibleInContainer } from 'foliate-js/paginator.js'; + +const rect = (left: number, top: number, width: number, height: number) => ({ + left, + top, + width, + height, + right: left + width, + bottom: top + height, + x: left, + y: top, + toJSON: () => ({}), +}); + +describe('isViewVisibleInContainer', () => { + const container = rect(0, 0, 1240, 600); + + it('returns true when the view fills the container', () => { + expect(isViewVisibleInContainer(rect(0, 0, 1240, 600), container)).toBe(true); + }); + + it('returns true for a long view spilling beyond the container (right column case)', () => { + // Mirrors the real-world case from issue #4243: the section starts at the + // right column edge (column ~= half container width) and extends far past + // the visible area on the right. + expect(isViewVisibleInContainer(rect(640, 0, 28000, 600), container)).toBe(true); + }); + + it('returns true for a view straddling the left edge', () => { + expect(isViewVisibleInContainer(rect(-100, 0, 200, 600), container)).toBe(true); + }); + + it('returns false for a view entirely off-screen to the left', () => { + expect(isViewVisibleInContainer(rect(-2000, 0, 600, 600), container)).toBe(false); + }); + + it('returns false for a view entirely off-screen to the right', () => { + expect(isViewVisibleInContainer(rect(1500, 0, 600, 600), container)).toBe(false); + }); + + it('returns false when the view touches the right edge but does not overlap', () => { + expect(isViewVisibleInContainer(rect(1240, 0, 600, 600), container)).toBe(false); + }); + + it('returns false when the view touches the left edge from the left but does not overlap', () => { + expect(isViewVisibleInContainer(rect(-600, 0, 600, 600), container)).toBe(false); + }); + + it('returns false when the view is fully above the container (scrolled-mode case)', () => { + expect(isViewVisibleInContainer(rect(0, -1000, 1240, 400), container)).toBe(false); + }); + + it('returns false when the view is fully below the container', () => { + expect(isViewVisibleInContainer(rect(0, 600, 1240, 400), container)).toBe(false); + }); +}); diff --git a/packages/foliate-js b/packages/foliate-js index 8e30a92d..1ea29964 160000 --- a/packages/foliate-js +++ b/packages/foliate-js @@ -1 +1 @@ -Subproject commit 8e30a92dbd93cb3e9c4289ba893dcbb57d71e77a +Subproject commit 1ea29964add965a228f8b771b4d22b418fb948d6