Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
jerome65536 opened this issue Nov 10, 2021 · 11 comments
Closed
Labels
2024 Update New discussions, issues and requests triggered by the 2024 update Proposed solution Issues or feature requests with a solution in reach

Comments

@jerome65536
Copy link

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:

  • Platform: iOS
  • Browser / Rendering Engine: iOS XX WKWebView

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
Copy link
Member

Isn't this the culprit?

:root[style*="readium-advanced-on"][style*="--USER__lineHeight"] {
line-height: var(--USER__lineHeight) !important;
}
:root[style*="readium-advanced-on"][style*="--USER__lineHeight"] body,
:root[style*="readium-advanced-on"][style*="--USER__lineHeight"] p,
:root[style*="readium-advanced-on"][style*="--USER__lineHeight"] li,
:root[style*="readium-advanced-on"][style*="--USER__lineHeight"] div {
line-height: inherit;
}

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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

@jerome65536
Copy link
Author

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
Copy link
Member

Ok thank you

@mickael-menu
Copy link
Member

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

/* Sanitize line-heights in webkit e.g. raised cap without a declared line-height
See effect by checking this demo in Safari: https://codepen.io/JayPanoz/pen/gRmzrE
Note: glyphs has to be reset to inline for CJK */
html {
-webkit-line-box-contain: block glyphs replaced;
}

@danielweck
Copy link
Member

Ah, good find Mickael! :)

@JayPanoz
Copy link
Collaborator

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 JayPanoz added 2024 Update New discussions, issues and requests triggered by the 2024 update Proposed solution Issues or feature requests with a solution in reach labels Apr 29, 2024
@JayPanoz
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2024 Update New discussions, issues and requests triggered by the 2024 update Proposed solution Issues or feature requests with a solution in reach
Projects
None yet
Development

No branches or pull requests

4 participants