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
89 stars 20 forks source link

-webkit-line-box-contain: block glyphs replaced; breaks line-height #113

Open jerome65536 opened 2 years ago

jerome65536 commented 2 years ago

I'm submitting a bug report

Short description of the issue/suggestion:

When there is a paragraph containing a long span, the line-height inside the span is too small. This happens even if explicitly setting style="line-height: 10.0" on the inner span.

Steps to reproduce the issue/enhancement:

  1. Make a paragraph containing a long (at least 4 lines) span.
  2. ?
  3. Look at the line height.

What is the expected behaviour?

Normal line-height everywhere.

What is the current behaviour?

Short line-height inside the span.

Do you have screenshots, GIFs, demos or samples which demonstrate the problem or enhancement?

Screenshot 2021-11-09 at 17 13 04

Note the especially small spacing between the 2nd and 3rd lines of the partially highlighted paragraph. The spacing between the 1st and 2nd, and between the 3rd and 4th lines seems slightly smaller, but it's a lot less noticable.

What is the motivation / use case for changing the behaviour?

For lines to be equally spaced

Do you know which CSS modules (stylesheets) are impacted?

ReadiumCSS-before.css

Please tell us about your environment:

Other information (e.g. related issues, suggestions how to fix, links for us to have context)

A workaround/fix is disabling -webkit-line-box-contain: block glyphs replaced; on the html element by setting the default -webkit-line-box-contain: block inline replaced; instead.

I couldn't find any documentation on what -webkit-line-box-contain: block glyphs replaced; actually means.

danielweck commented 2 years ago

Isn't this the culprit? https://github.com/readium/readium-css/blob/583011453612e6f695056ab6c086a2c4f4cac9c0/css/dist/ReadiumCSS-after.css#L726-L735

Take a look at the Web Inspector / CSS debugger, and find the style cascade for your span element, compare it with the other HTML element which has the expected line height.

jerome65536 commented 2 years ago

Even if setting the line-height manually so that the span element cascade shows a line-height: 4 in the inspector, it still displays as if it was line-height: 1. The only thing I found that fixes the effective line-height (which is different than what the inspector shows) is disabling the undocumented -webkit-line-box-contain: block glyphs replaced; either via the inspector or some other way such as via Javascript.

Unless setting it to display: inline-block, that is, in which case the line-height displays correctly, but the highlighted text is then disconnected from the rest of the paragraph, so that way isn't useful.

danielweck commented 2 years ago

Just to clarify: are you sure that the line-height you "force" via Web Inspector / CSS debugger has the highest precedence in the style cascade (i.e. selector specificity, !important etc.)?

jerome65536 commented 2 years ago

Yes, I've tried via style="line-height: 4 !important" even though style="…" should always have the highest specificity.

And even without explicitly setting the line-height at all, the inspector says theline-height is the same as the other paragraphs, it's just rendered wrong.

Disabling -webkit-line-box-contain: block glyphs replaced; doesn't change the computed line-height according to the inspector, it just somehow affects the actual rendered line-height. I don't know if it's following the spec, since there doesn't even seem to be any spec.

danielweck commented 2 years ago

Just out of curiosity, Chrome / Chromium (Blink) or Safari / WebKit? What about Firefox?

jerome65536 commented 2 years ago

I can't reproduce anywhere except in the iOS WKWebView, even copying/pasting the HTML/CSS from the inspector.

It says somewhere that the deprecated -webkit-line-box-contain is only implemented in Safari (maybe has been in some other browsers, but got removed). As far as I can tell, it's only in the iOS WKWebView, though.

danielweck commented 2 years ago

Ok thank you

mickael-menu commented 2 years ago

For reference, here's where this is set and the associated comment:

https://github.com/readium/readium-css/blob/583011453612e6f695056ab6c086a2c4f4cac9c0/css/src/modules/ReadiumCSS-safeguards.css#L26-L31

danielweck commented 2 years ago

Ah, good find Mickael! :)

JayPanoz commented 2 months ago

To add some extra context, that was added at the time in order to keep consistent with iBooks and not surprise authors, should they happen to find themselves in the situation of a raised cap w/o a declared line-height while proofing/debugging their files.

That being said, maybe keeping things consistent within ReadiumCSS-based apps is more important, as this is only applicable to Safari/iOS and there is no such safeguard for Chromium/Android. In that case it should be removed entirely.

JayPanoz commented 1 month ago

Proposed resolution: this should be removed in order to provide more consistency.