image scaling (fit into viweport), cover pages (HTML), surrounding markup, authored CSS, etc. (causes for unnecessary scroll extent / overflow due to margins-paddings)

[danielweck]

Issue originally discussed at: https://github.com/readium/r2-navigator-js/issues/18

[danielweck]

Insightful comments by @JayPanoz starting here: https://github.com/readium/r2-navigator-js/issues/18#issuecomment-444148032

[danielweck]

Possibly related: https://github.com/readium/readium-css/issues/50 https://github.com/readium/readium-css/issues/10

[JayPanoz]

As regards “possibly related”.

• 50: this sample is really, really, a stink bomb in the sense it “implements” one of the worst edge cases possible. Nothing personal to the people who made it, obviously, but this is a case that is not even discussed in the EPUB spec i.e. publications with documents in both horizontal and vertical writing. It’s been popping up over and over and over again but the cover styling doesn’t even work properly in a “vanilla” web browser – I checked that like 5 times minimum, with Readium CSS, without Readium CSS, without any EPUB logic, etc., and it failed every time. That being said, it’s kinda my fault, I should have opened an issue about this sample long ago.

• 10 is about how the viewport is altered, and how that impacts viewport relative lengths. When using iframe, you can for exemple set an explicit height and all of a sudden, 100vh can become 15000px instead of being the browser’s window’s 860px because it’s the height of the iframe that counts for the viewport in the EPUB styling context. I can confirm that is something that shouldn’t happen when using web views.

So to sum up, this issue can be scoped to the additional margin we’re setting up in scroll mode.

https://github.com/readium/readium-css/blob/051e3d2d18423c1433964a02816f5d93364ec124/css/src/modules/ReadiumCSS-scroll.css#L24-L30

Thanks for opening this issue BTW. 🙏

[danielweck]

I can confirm that is something that shouldn’t happen when using web views.

:)

...well, unless the said "web view" is Electron's own interpretation/implementation (derived from Chromium webview) which is in fact an embedded object (custom element + shadow DOM) that emulates a sandboxed iframe (isolated in a separate process), so vh and vw may in fact apply (to be honest, I haven't checked thoroughly).

See: https://electronjs.org/docs/api/webview-tag ... I know, there is an alarming disclaimer at the top, but readium-desktop, r2-testapp-js and of course r2-navigator-js all build on top of Electron 1.* which isn't so deprecating about webview: https://github.com/electron/electron/blob/v1.8.8/docs/api/webview-tag.md We are tracking this problem for later: https://github.com/readium/r2-navigator-js/issues/15

[JayPanoz]

Right, I’ll follow up tomorrow as I’m quite frankly exhausted right now and some comments may appear passive-agressive – sorry about that, wasn’t my intention – but we’d better have a solid reason for changing how that works, especially as it’s in the 2017 CSS snapshot, on which EPUB 3.2 is now relying – and well, if I’m being honest and although I’m trying really hard to be neutral, it’s very difficult to not be on some kind of a mission as I personally saw the nicest and most talented people quit ebooks because ebooks break such standards and they couldn’t take it anymore.

For vw (viewport width) I’m totally cool saying “Hey, don’t count on that, it’s not compatible with CSS multicol, that we’re using for pagination.” For vh though, I’m super reluctant, as this is quite possibly the only way out for styling covers as far as authors are concerned – and we’re also using that actually, in multiple places – and spoiler alert: authors are primarily writing their styles for pagination, most won’t even test their ebooks in scroll.

At first I thought it would maybe impact scroll-continuous, but this isn’t necessarily the case – and the scope may be well wider. So I’ll take a look tomorrow, thanks for the links BTW.

[JayPanoz]

That + some vendors advise vh actually so people are already following those guidelines.

In Flowing EPUB books, it is recommended to size images using viewport units to maintain adaptability for various screen sizes.

Source: https://help.apple.com/itc/booksassetguide/#/itca71ad3c33

[JayPanoz]

But then again, note the primary issue is about adding a margin-top and margin-bottom in scroll mode so that text content is not “glued to the edges” of the RS there.

Works for text but not for files consisting of images only. So I’d really like to scope this issue to that and see what our options are – not so much using CSS only unfortunately, but it’s worth discussing imho.

[JayPanoz]

Alright, here’s an illustrated recap on scroll styling.

So right now, we have the following box model for body:

Don’t pay attention to horizontal margins (372, we actually don’t set this one, our value is auto). Other than that:

1. body has vertical margins of var(--RS__pageGutter) so 50px here ;
2. body has horizontal padding of var(--RS__pageGutter) so 50px too;
3. the “content container” is therefore 540px wide.

But why did we set vertical margins?

It was all about text.

Note: In the following screenshots, orange (outer) is body’s margins, green (in-between) is body’s padding, and blue (inner) is “content container” (div, figure, etc.)

For instance, here’s a chapter without any margin-/padding-top.

I can recall a heading in our test files that was somehow partly hidden at the top of the chapter because we didn’t apply any margin. I can’t remember why exactly (maybe negative margin-top, relative positioning, or the iOS navbar hiding part of it only in the scroll view) but we added that as some sort of safeguard so that it can’t happen.

And same for the bottom of chapters.

Without a margin

With a margin

So the idea was to give some extra vertical spacing for content to breathe.

Let’s now look at images (e.g. cover).

Without a margin (note we constrain the height to 95vh (so 95% of the viewport’s height so that we don’t accidentally create blank pages in the paged view).

With a margin

I can confirm there’s overflow in this last screenshot, although you can’t see the scrollbar (because MacOS is buggy). But now the height of the document is the height of the image (95vh) + the margins (50px * 2) so it is obviously overflowing.

This is obviously something we don’t want, we want no extra vertical margins there.

What the issue is

In CSS, you can’t conditionally apply margins depending on the document’s content (text vs. only one image). I indeed can’t use such logic:

if body has only an image
then remove margin-top and -bottom from body

Because when checking this image, I’m already getting out of body’s scope, I’m further down the DOM (the image) and I can’t “bubble up” i.e. styling its parent elements.

All I can do is expecting some kind of flag, so in ReadiumCSS by default, a custom property such as style="--RS__imgOnly: true" either on html or body e.g.

:root[style*="--RS__imgOnly: true"] body {
  margin-top: 0;
  margin-bottom: 0;
}

/* Note that could be [style*="--RS__imgOnly: false"] but since `--RS__imgOnly` is a boolean, no need to use it */
:root body {
  margin-top: var(--RS__pageGutter); /* 50px */
  margin-bottom: var(--RS__pageGutter); /* 50px */
}

So all we can do with vanilla CSS is either applying vertical margins to all documents or not applying them.

Hypothetical fixes

Such logic is JS territory, either in the form of:

1. an :has() (CSS pseudo-class) polyfill – but brace yourself for the handling of all author markups possible;
2. a logic that checks what is in the document and apply a flag to html or body, and that I can use in CSS to target body (could also be parser and not JS at runtime per se).

[JayPanoz]

Related: scrolled-continuous rendition e.g. how will it be implemented, will it break vh, how to make a clear distinction between files, etc.?

[JayPanoz]

After further reflection, I’m wondering whether rolling back to no vertical margins and letting the app handle those, as in the paged view, wouldn’t be the most reasonable call. That would enforce consistency for implementers, which in my opinion is most important.

[ronghester]

I have observed a similar stretching of cover image issue with ebook. The cover image is rendered from start.html and since it is provided by the publisher it can have different html and css properties. For example i am showing below the html of two ebooks where in first book the cover image rendered perfectly but in other it gets stretched to multiple pages.

HTML in start.html where cover image does not stretch.

<body style="margin:0.00em; text-align:center;">
   <p class="cover" style="text-align:center;"><img style="height: 100%;" src="9780857633446_cover_epub.jpg" alt=""/></p>
 </body>

Cover image is stretch in this book

<body style="margin-top:0px; margin-left: 0px; margin-right: 0px; margin-bottom: 0px;  text-align: center; background-color: #ffffff;">
 <div><img src="docimages/cover_ader.jpg" alt="cover" style="height: 100%" /></div>
</body>

Apparently we have observed this issue when migrated to recent readium-sdk along with LCP. In the previous setup which was using 2-3 yr old readium this problem was not so much evident.

We are using readium on android & IOS platform.

Is there any easy fix for this ?

Thanks

[danielweck]

we have observed this issue when migrated to recent readium-sdk

Just to be clear: you mean Readium2, where ReadiumCSS is used to apply document formatting, right? (I am asking just in case you use a hybrid Readium1 + ReadiumCSS solution)

[ronghester]

We are using Readium1 from this branch : https://github.com/readium/readium-sdk/tree/feature/latest-working-build-config/Platform/Android

[danielweck]

Oh, so you are not using ReadiumCSS, right?

[danielweck]

For Readium1, please report in readium-shared-js, e.g.

https://github.com/readium/readium-shared-js/issues/342

https://github.com/readium/readium-shared-js/blob/7f245beba1ed97eaabce0aa5e9cf2f3b23e8f8f6/js/views/reflowable_view.js#L988-L1018

(the image handling code in Readium1 SDK has not changed for months/years, but your migration from a very old revision of Readium1 may indeed have introduced different behaviours / regression bugs)

[ronghester]

Yes we are not using ReadiumCSS in the current setup. Also we were not using it earlier. In asset folder there are some css file present such as SDK.css etc.. I believe readiumCSS is applicable only to readium 2 and onward, right?

Thanks i will report in readium-shared-js project.

[JayPanoz]

So yeah, readiumCSS is applicable to r2 onwards.

Note we have a list of CSS-related issues from r1 that this project is attempting to fix in the wiki: https://github.com/readium/readium-css/wiki/CSS-related-issues-in-Readium-1 – uncrossed items don’t necessarily mean we haven’t fixed it, maybe it doesn’t apply, or we haven’t had the opportunity to address it yet, etc.

[JayPanoz]

cc @danielweck Removed those vertical margins in develop branch. Please let me know if this fix the overflow: scroll issue for covers.

[danielweck]

Thanks Jiminy, I've merged your updates into the ReadiumCSS copy of r2-navigator-js: https://github.com/readium/r2-navigator-js/commit/4d956424ddaa4eec6c52516176c1deeb7f66dc80

Test with Metropolis (which I found in your screenshots https://github.com/readium/r2-navigator-js/issues/18#issuecomment-444162879 ), from Feedbooks: http://www.feedbooks.com/book/1123/metropolis

Load the remote EPUB directly in r2-testapp-js using the command line: npm run electron http://www.feedbooks.com/book/1123.epub.

Cover and text in paginated mode (CSS columns):

Cover and text in scroll mode:

As you can see, no extraneous scroll extent, thanks to the removed top/bottom margins in ReadiumCSS.

[JayPanoz]

No problem, and thanks for the confirmation.

Will resolve this issue when merging this change.

[JayPanoz]

Fixed via #63