readium-css icon indicating copy to clipboard operation
readium-css copied to clipboard

EPUB with `overflow-x: hidden` style breaks pagination

Open mickael-menu opened this issue 3 years ago • 12 comments

This epub has overflow-x: hidden CSS for the <body> tag. That is what is causing this. @mickael-menu should Readium CSS be changed to always override that?

Originally posted by @stevenzeck in https://github.com/readium/kotlin-toolkit/issues/292#issuecomment-1301535645


Original Bug Report by A-Fawzyy

What happened?

The attached epub's chapter shows in a single page, where it should show over several pages.

https://user-images.githubusercontent.com/38096011/199537331-a167f47a-0f17-4765-8b07-a173600f3432.mp4

Expected behavior

Only a part of the chapter should show, and the rest of the chapter should be displayed after navigating to the next page.

Actual behavior

The chapter is rendered in a single page.

How to reproduce?

‎⁨الحرب_والسلم_(الكتاب_الأول)⁩.epub.zip The ebup download url: https://downloads.hindawi.org/books/26307461.epub

Environment

  • Readium version: 2.2.1

Development environment

  • OS: macOS Ventura 13.0
  • IDE: Android Studio Dolphin | 2021.3.1 Patch 1

Testing device

  • Android version: 11
  • Model: Realme Narzo 50i
  • Is it an emulator? No

Additional context

  • Are you willing to fix the problem and contribute a pull request? Yes, Please

mickael-menu avatar Nov 03 '22 07:11 mickael-menu

I tested الحرب_والسلم_(الكتاب_الأول).epub with Thorium. The page renders fine, I think:

Screenshot 2022-11-03 at 16 05 58

...but, Thorium injects additional stylesheets immediately after the ReadiumCSS "after" layer (which itself comes after publishers styles, which itself comes after ReadiumCSS "before" layer). The injected CSS is the product of incremental findings following weird edge-case bug reports (there is still a known problem with some HTML pages that don't extend the full height, so it is not "perfect" yet). The CSS bugs are super tricky to reproduce, sometimes impossible from the Web Inspector (must author the CSS at source and do a hard page reload, can't dynamically change properties in the style debugger).

For what it's worth, here is the current stack of CSS overrides in Thorium:

/*
https://github.com/readium/readium-css/issues/117
no new stacking context, otherwise massive performance degradation with CSS Columns in large HTML documents
(web inspector profiler shows long paint times, some layout recalc triggers too)
*/
:root {
    -webkit-perspective: none !important;
    perspective: none !important;
}

:root[style].r2-css-paginated:not(.r2-fixed-layout),
:root.r2-css-paginated:not(.r2-fixed-layout) {
    overflow: visible !important;
}
:root[style].r2-css-paginated:not(.r2-fixed-layout) > body,
:root.r2-css-paginated:not(.r2-fixed-layout) > body {
    overflow-x: hidden !important;
    overflow-y: visible !important;
}

:root[style].r2-fixed-layout,
:root.r2-fixed-layout {
    overflow: hidden !important;
}
:root[style].r2-fixed-layout > body,
:root.r2-fixed-layout > body {
    overflow: hidden !important;
    margin: 0 !important;
}

:root.r2-css-paginated > body,
:root:not(.r2-css-paginated) > body,
:root.r2-fixed-layout > body,
:root:not(.r2-fixed-layout) > body,
:root[style].r2-css-paginated > body,
:root[style]:not(.r2-css-paginated) > body,
:root[style].r2-fixed-layout > body,
:root[style]:not(.r2-fixed-layout) > body {
    position: relative !important;
}

:root[style]:not(.r2-css-paginated):not(.r2-fixed-layout),
:root:not(.r2-css-paginated):not(.r2-fixed-layout) {
    height: 100vh !important;
}

:root[style]:not(.r2-fixed-layout) > body,
:root:not(.r2-fixed-layout) > body {
    min-height: inherit;
}
:root[style]:not(.r2-css-paginated):not(.r2-fixed-layout) > body,
:root:not(.r2-css-paginated):not(.r2-fixed-layout) > body {
    height: inherit;
}

danielweck avatar Nov 03 '22 16:11 danielweck

Ah, I disabled the CSS hacks (see stylesheet snippet above), and the Arabic RTL still text flows correctly in Thorium. I guess the Chromium web browser engine is happier in Thorium than in Android? :)

danielweck avatar Nov 03 '22 16:11 danielweck

Thanks for the feedback @danielweck. I tested on iOS and it worked fine as well, so it might be an Android-specific issue.

Interestingly the edge taps work on Android, it's only the swipes that fail. Might be an issue in the gesture recognition.

mickael-menu avatar Nov 03 '22 16:11 mickael-menu

What the hell?! Thorium is broken with latest Electron build (updated Chromium). Same pagination problem with overflow X

danielweck avatar Nov 15 '22 17:11 danielweck

😅 This is what I injected to fix the issue, inspired by your snippets:

:root[style], :root { overflow: visible !important; }
:root[style] > body, :root > body { overflow: visible !important; }

mickael-menu avatar Nov 15 '22 18:11 mickael-menu

Unfortunately I cannot use that trick in Thorium :(

(see the extraneous scroll bars!)

Screenshot 2022-11-15 at 18 42 46

danielweck avatar Nov 15 '22 18:11 danielweck

overflow-x: clip seems to work ... testing

danielweck avatar Nov 15 '22 18:11 danielweck

clip works (including RTL text flows), but breaks with last chapter of EPUB "Fundamental Accessibility Tests: Visual Adjustments" ("Supplemental Content" HTML).

UPDATE: the culprit is overflow: hidden; in the child section HTML element of body...not sure the reading system should try to be smart in this case!? (e.g. inject CSS selectors with higher specificity to override publisher styles deep in the style cascade, or even just at the immediate child level of body, at the risk of breaking something else!)

danielweck avatar Nov 15 '22 18:11 danielweck

@danielweck I think the epub in the kotlin-toolkit issue has the issue directly in the <body> tag. So this may require a more extensive long-term fix.

I looked through a few Chromium issues, but nothing stood out as to why this started happening.

stevenzeck avatar Nov 16 '22 00:11 stevenzeck

Let's reopen this issue until we have a fix suitable for all platforms.

mickael-menu avatar Nov 16 '22 08:11 mickael-menu

TS toolkit: https://github.com/readium/ts-toolkit/blob/dev/navigator-html-injectables/src/modules/snapper/ColumnSnapper.ts#L206-L222

danielweck avatar Nov 16 '22 18:11 danielweck

Ouch, there is a "regression" bug (sort of) in cover images that used to span across into the next CSS column:

https://github.com/edrlab/thorium-reader/issues/1861#issuecomment-1328089302

Damn.

danielweck avatar Nov 26 '22 18:11 danielweck