readium / readium-css

🌈 A set of reference stylesheets for EPUB Reading Systems, starting with Readium Mobile
https://readium.org/readium-css/
BSD 3-Clause "New" or "Revised" License
90 stars 20 forks source link

Text rendering issue with readium's css #95

Open rahulk11 opened 3 years ago

rahulk11 commented 3 years ago

I am having issue with readium css in r2 test app for some epubs. The text is overlapping in some cases and in some cases text on top is getting cropped. I have included the screenshots for all the cases and also from readium1 test app(SDKLauncher-Android) It mostly works fine in readium1.

Case: publisher's default turned off

1.

publisher's default turned off1

2.

publisher's default turned off2

3.

publisher's default turned off3

Case: publisher's default turned on

1.

publisher's default turned on1

2.

publisher's default turned on2

3.

publisher's default turned on3

Case: Readium1 Test App

1.

readium1_test_app1

2.

readium1_test_app2

3.

readium1_test_app3
danielweck commented 3 years ago

Hello, can the problem be reproduced consistently in all R2 target platforms? (iOS, Android and desktop/Electron)

rahulk11 commented 3 years ago

Hi @danielweck , yes, the issue is reproducible in iOS as well as thorium - r2 based desktop app.

This is in iOS CA44CA4E-6447-4E03-80C0-BC6E236C17BB_20201012_140501924.jpg

And this in thorium Screenshot 2020-10-12 at 12.09.41 PM_20201012_135952916.jpg

JayPanoz commented 3 years ago

This indeed is a side-effect of disabling publisher’s settings as we then apply a font-normalisation to handle font-size. That font-normalisation exists because we don’t have a universal way/API to handle font-size in browsers, on all platforms, and a significant percentage of EPUB files would have otherwise broken the font-size setting due to the usage of absolute units. It’s the best we can do in CSS due to the lack of such standard feature.

This should serve as a use-case for #34 though, because we’ve been willing to strongly emphasise that if a native font-size API/method is available on a particular platform, it should be used in lieu of Readium CSS’ font-normalisation.

rahulk11 commented 3 years ago

@JayPanoz Hi, There are issues also with the publisher's default ON. Check above screenshots . The texts are overlapping.

JayPanoz commented 3 years ago

Thanks for your patience.

I’ll have to coordinate with @danielweck and @mickael-menu on this one, as I’m not sure what is exactly going on in the apps.

So as a recap, these headings are p with classes, which is the reason the impact with publisher’s default off is so huge – should these be h1, h2, etc. it would already solve some rendering issues with advanced settings. That’s also why the chapter heading – which is using a table with a negative bottom margin for layout purposes – ends up being cut off with publisher’s default off.

Anyways.

Thorium:

I am able to consistently reproduce these rendering issues with the test app as well when enabling settings, but they disappear w/o any issue when re-enabling publisher’s default. I can also tell they happen because readium-advanced-on is set on the root element (+ text-align).

What I don’t understand is why it stays there after a single setting change, @danielweck? Or does its removal not trigger a relayout?

Android + iOS:

So this overlapping text with publisher’s default on left me dumbfounded to be honest. Technically, ReadiumCSS does set a line-height on html just in case, knowing the vast majority of authors’ stylesheets will override that value further down the DOM (body, h1, p, etc.). This is not the case with this EPUB.

It should also be large enough that this overlapping doesn’t happen, and I can’t really understand the discrepancy between Android and iOS so I’ll need @mickael-menu’s help on this one.

Android: it clearly overlaps.

Screenshot_20201023-104618

iOS: it’s close, mind you, but it doesn’t.

Photo 23-10-2020 10 15 41

There difference might be explained by the computed value of line-height for html and/or if it is inherited by this p? At first I thought that maybe it could be a bug in Chrome re. font metrics but having tested the iOS test app I’m not so sure now.

In passing, such “freeform” heading should really really set a line-height, because there is no guarantee the default will be the same in all apps – in some, users can even change this default.