Repository navigation
Fix/epub reader layout reflow - #402
Open
JimmyJi0309 wants to merge 4 commits into
Open
JimmyJi0309 wants to merge 4 commits into
JimmyJi0309 wants to merge 4 commits into
Conversation
- Recreate the EPUB reader once after layout-critical reader settings are changed and the options sheet is closed, avoiding a stale Readium page offset. - Reserve the optional title/author header space in the EPUB reader padding path, matching the PDF and comic readers. - Add a JVM test for the layout-change predicate.
Address review follow-ups: include header/progress padding changes in the relayout predicate, recompute padding when header insets change, skip recreation while showing chrome, and avoid recreating for reading-direction-only changes.
The supplied EPUB has a 1026x1465 inline image that can be laid out at its intrinsic width and split across two phone pages. Normalize large images/SVGs to the current page/column width and height after load/resize, leaving small icons alone.
Do not normalize images until the WebView has a measurable column width, and leave an 8px edge gap so a rounded image width cannot paint a stray vertical line at the page edge.
Contributor
|
The layout reflow issue was on my list too. 😄 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix two independent EPUB-reader problems reported on a phone:
The tablet rendering was unaffected.
Cause
Illustration split
A reflowable EPUB with a large inline illustration (roughly 1026 × 1465, plain
<p><img>with no publisher CSS, float, or fixed dimensions) exposed the issue. On the narrower phone page, the image was laid out wider than one rendered page/column and Readium split it across pages. The first normalizer attempt also ran before the WebView had a measurable width and could compute a 1px image width, producing a stray vertical edge line.Typography layout
Readium applies typography through a live
submitPreferencesCSS reflow, but its pagination is index-based. After a large reflow it keeps the old page index instead of re-anchoring the locator, so the visible page is temporarily wrong. Reopening recreates the navigator and paginates once from the saved locator, which is why close/reopen fixed it.A separate EPUB defect was also fixed: unlike the PDF and comic readers, the EPUB reader did not reserve space for the optional title/author header, so the header could overlay content.
Changes
epubLayoutChangeRequiresRelayout(...)covering font size/family, spacing, alignment, hyphenation, page padding, header visibility, progress-footer visibility and layout mode. Reading-direction-only changes are excluded because EPUB reading progression is fixed to LTR.applyReaderPadding()path using the samereaderHeaderReservedTopSpace(...)helper as PDF/comic, and re-apply padding after header/inset changes.Verification
<p><img>(about 1026×1465) and no image CSS; the fixture is not attached to keep the PR small../gradlew testDebugUnitTest lintDebug assembleDebug assembleDebugAndroidTest— passedpublisherStyles,shouldApplyInsetsPadding, or the legacy WebView reader path.Notes
The EPUB fixes are independent of the reader-settings feature branch. The typography fix intentionally recreates once when the options sheet is closed; changing typography and continuing to read with the sheet open may still show the live reflow until the sheet is dismissed.
Close #401