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

Book cover has strange flow and cut into 2 pages #50

Closed iaomw closed 5 years ago

iaomw commented 6 years ago

When using RLT css, this book cover is cut off and has strange flow. Please check this link for testing files. https://github.com/readium/swift-toolkit/issues/185 As shown in the picture below, It could be reproduced with Safari on Mac. It has the same effect in our app using WKWebView. It should looks good in WKWebView. This is a SVG node inside h1 node. If I move everything from h1 to body, it will be fixed. So, there might be some problem with h1 styles.

untitled 8
iaomw commented 6 years ago

@JayPanoz

JayPanoz commented 6 years ago

Hmmm, I can see two issues in <head> at first sight:

  1. ReadiumCSS-before.css is loaded after all other stylesheets while it should come first (before authors’ stylesheet(s);
  2. the ReadiumCSS stylesheet used are the ones for CJK in vertical writing (for RTL, they should be the one in the RTL folder, which is this one)

Do you get the same result with the RTL stylesheets?

JayPanoz commented 6 years ago

Also, I would recommend dividing this sample into 2 because, in and of itself, it exactly is an edge case we agreed to not handle back in December 2017 as the EPUB spec provides zero guidance for it.

So it would be a better idea to isolate Japanese from Arabic/Hebrew.

iaomw commented 6 years ago

Hi, the order doesn't matter, it will have the same effect. I have double checked it in Safari.

This one in picture is modified by myself on Mac, so it has this order. Don't worry. The webpage generated by Streamer has correct order.

The book is using CJK vertical mainly, and this cover html is mark as xml:lang="ja" Logically, we should use CJK vertical. What is the reason to use others?

I just checked the css file under rtl, it will have some difference but still been cut off.

JayPanoz commented 6 years ago

OK I’ll take a look and report back.

But if we end up discovering we need to isolate h1 wrapping svg or images from others, there’s nothing we can do in CSS to achieve that.

iaomw commented 6 years ago

Thanks. I also found if the h1 has a width:(some number smaller than the container): important! It will also work fine. But I am not CSS expert. You guys should has better solutions.

JayPanoz commented 6 years ago

OK so I was able to reproduce this issue… without loading any ReadiumCSS stylesheet.

capture d ecran 2018-05-18 a 17 38 52

h1 indeed has no dimensions set (width and height) so it can’t constrain the nested SVG which is 100% of… nothing. This explains why putting a width on h1 works, as you reported in the comment above.

With ReadiumCSS Stylesheets loaded we got this (note columns are on the y-axis).

capture d ecran 2018-05-18 a 17 35 29

capture d ecran 2018-05-18 a 17 35 36

I’m reluctant to safeguard h1 there, as some may well be more than the width of the page so our best option is probably switching to viewport width (vw) there. But that is not a perfect solution as it will indeed create a blank page after.

I’ll try deploying the change on the develop branch ASAP so that we can see if it is good enough. But there’s not much we can do considering it doesn’t even work properly when kept untouched (and there’s consequently an authoring issue).

llemeurfr commented 6 years ago

As said before to @iaomw, this edge case EPUB sample must be put on the side for RTL study and only used for check if the issue we see here can be solved easily. We must not spend too much time on this.