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

C++ syntax highlighting on iOS html #7248

Closed
jonathanreeves opened this issue Apr 27, 2021 · 15 comments
Closed

C++ syntax highlighting on iOS html #7248

jonathanreeves opened this issue Apr 27, 2021 · 15 comments

Comments

@jonathanreeves
Copy link

I have a document that I render into a few different formats, including html. The html output looks great on most platforms, but on iOS, all syntax-highlighted C++ is badly mangled. Screenshot attached to show what I mean.

pandoc-screenshot

Relevant info:

jon@ubuntu:pandoc-debug$ pandoc --version
pandoc 2.13
Compiled with pandoc-types 1.22, texmath 0.12.2, skylighting 0.10.5,
citeproc 0.3.0.9, ipynb 0.1.0.1
User data directory: /home/jon/.local/share/pandoc
Copyright (C) 2006-2021 John MacFarlane. Web:  https://pandoc.org
This is free software; see the source for copying conditions. There is no
warranty, not even for merchantability or fitness for a particular purpose.

Minimal test case:

UserGuide.md

Rendered with the following:

pandoc UserGuide.md -s -o UserGuide.html

I tested with both Safari and Chrome on an iPhone running iOS 14.4.2. Same behavior in both browsers, so this seems to be a platform-wide thing. Since it's only iOS, I'm guessing the most obvious answer is that this is a problem in Apple's html rendering in iOS. If that's true, it would at least be helpful to narrow down what's going wrong to put together a better bug report for them.

@mb21
Copy link
Collaborator

mb21 commented Apr 27, 2021

I can confirm this on iOS 14.4.2 and 14.5 as well (no, -M document-css=false doesn't help).
Furthermore, everything looks fine on desktop Safari 14.0.3 and 14.1.

@mb21
Copy link
Collaborator

mb21 commented Apr 27, 2021

Interestingly, removing the following CSS rule fixes it:

pre > code.sourceCode > span {
  display: inline-block;
}

source of that seems to be this line, which has been there for more than two years...

@jonathanreeves
Copy link
Author

Thanks! I can confirm that this fixes the mangling, both on the minimal test case and in my original full doc. Of course not surprisingly it also causes the lines to wrap instead of allowing you to scroll left and right. This is probably still strictly better, but it would be nice to be able to maintain the scrolling behavior on mobile devices and avoid wrapping. At the very least this helps a lot with figuring out where mobile Safari is losing its mind.

@jgm
Copy link
Owner

jgm commented Apr 27, 2021

I think we want the non-wrapping behavior to be the default, as wrapping can lead to problems in code (leading people to think the code contains a line break). So, it looks like an iOS bug, and maybe this can be closed?

@mb21
Copy link
Collaborator

mb21 commented Apr 28, 2021

Definitely looks like an iOS bug. Unfortunately for us, that has quite high usage-share. So if they don't fix it soon, and it happens often (not just with this specific piece of code highlighted in cpp), then we may have to find a work-around... like in the good old Internet Explorer days 🙈

@jonathanreeves
Copy link
Author

For what it's worth, I submitted a bug report to Apple, but that's always kind of a black box. I have the test cases hosted here:
https://mimicc.dev/ios-good.html
https://mimicc.dev/ios-bad.html

As someone who briefly worked at Apple, the best way to get these things fixed is always to talk to someone who works on the team in question. Unfortunately I don't know anyone on the mobile Safari team, but maybe someone else here does?

Also not sure if this is helpful, but the original context of the document where this came up is here (already corrected to remove the inline-block):
https://mimicc.dev/user-guide/

Really appreciate the work you guys do, having a tool like this has been invaluable for writing and distributing documentation.

@mb21
Copy link
Collaborator

mb21 commented Apr 28, 2021

As someone who briefly worked at Apple, the best way to get these things fixed is always to talk to someone who works on the team in question. Unfortunately I don't know anyone on the mobile Safari team, but maybe someone else here does?

Nope, I only follow @jensimmons on twitter 😅
But do you have a radar nr?

@jgm
Copy link
Owner

jgm commented Apr 28, 2021

@mb21 can you think of a workaround that would make sense, short of just setting the default to wrap (which I don't want to do)? Is there a way to set the default to wrap on mobile devices, or on IOS, for example?

@jonathanreeves
Copy link
Author

Nope, I only follow @jensimmons on twitter 😅
But do you have a radar nr?

Just the one assigned through feedback assistant: 9091417

@jgm
Copy link
Owner

jgm commented Apr 28, 2021

Maybe I'll leave this open in case someone can find a good workaround.
It may be some time before the bug is fixed, after all.

@mb21
Copy link
Collaborator

mb21 commented Apr 29, 2021

I had a quick look, and couldn't find an easy fix. Probably could get something working with either css grid or flexbox, e.g:

pre > code.sourceCode {
  display: grid;
}

but that wouldn't work for older browsers and really would be a rewrite of all that... so let's hold off doing that for a bit, maybe Safari will have it fixed in the next release...

@AlbertLei
Copy link

Can confirm this bug still exists on iOS 16.1.1 when using pandoc 3.1.4

@jgm
Copy link
Owner

jgm commented Aug 27, 2023

Since this bug in iOS is obviously not a priority for them, maybe we should capitulate and remove

pre > code.sourceCode > span {
  display: inline-block;
}

or make it conditional so that it doesn't get activated for mobile devices. Someone who really knows CSS may have other ideas...

@jgm
Copy link
Owner

jgm commented Aug 27, 2023

In my tests, removing the display: inline-block has no bad effects; lines still scroll rather than wrapping...so I will do this.

@jgm jgm closed this as completed in 18bf7e1 Aug 27, 2023
@AlbertLei
Copy link

Can confirm removing display: inline-block "solves" this iOS bug without introducing other noticeable changes.
Thanks!

jgm added a commit to jgm/skylighting that referenced this issue Feb 28, 2024
See jgm/pandoc#7248 for an explanation of why this was removed
from the regular CSS, and see jgm/pandoc#9520 for an explanation
of the problems this causes in printed output.
jez added a commit to jez/skylighting that referenced this issue Nov 18, 2024
Having `display: inline-block;` is useful. I frequently customize the
pandoc code block styles to allow for line highlights (see [1]).

The CSS styles for those line hightlights rely on the enclosing `span`
for a line being able to span the full width of the code block, which is
not possible without making the `span` be `display: inline-block;`.

The original issue reported in jgm/pandoc#7248
was that specifically iOS Safari renders font sizes incorrectly.

This is a known iOS bug. I first discovered the workaround for it while
working with the tufte-css project (see [2]), and similar workarounds
are implemented in various CSS normalize/reset projects.

I have verified that including this `-webkit-text-size-adjust` property
fixes the original bug reported against pandoc. Given that the solution
in this PR allows keeping `display: inline-block;` which has its own
uses, fixes the original bug, and is an iOS-specific solution to an
iOS-specific problem, I believe that this is a more compelling long-term
solution (at least until iOS decides to change this behavior).

I, of course, defer to the maintainers of the skylighting project as to
whether you want this change. I've already worked around this problem in
my own projects that consume the output of pandoc- and
skylighting-highlighted code blocks (by basically vendoring these
changes into those projects), so I am not personally blocked. I'm
contributing this change upstream in the hopes that it might help
unblock others.

[1]: https://jez.io/pandoc-markdown-css-theme/features/#numbered-and-highlighted-lines
[2]: edwardtufte/tufte-css#81 (comment)
jgm pushed a commit to jgm/skylighting that referenced this issue Nov 19, 2024
* Revert "Re-add `display: inline-block`, but only to print CSS."
  This reverts commit 15682b2.

* Revert "Remove display: inline-block from span CSS."
  This partially reverts commit
  0ec8892

* Use `-webkit-text-size-adjust` instead of unsetting inline-block

Having `display: inline-block;` is useful. I frequently customize the
pandoc code block styles to allow for line highlights (see [1]).

The CSS styles for those line hightlights rely on the enclosing `span`
for a line being able to span the full width of the code block, which is
not possible without making the `span` be `display: inline-block;`.

The original issue reported in jgm/pandoc#7248
was that specifically iOS Safari renders font sizes incorrectly.

This is a known iOS bug. I first discovered the workaround for it while
working with the tufte-css project (see [2]), and similar workarounds
are implemented in various CSS normalize/reset projects.

I have verified that including this `-webkit-text-size-adjust` property
fixes the original bug reported against pandoc. Given that the solution
in this PR allows keeping `display: inline-block;` which has its own
uses, fixes the original bug, and is an iOS-specific solution to an
iOS-specific problem, I believe that this is a more compelling long-term
solution (at least until iOS decides to change this behavior).

I, of course, defer to the maintainers of the skylighting project as to
whether you want this change. I've already worked around this problem in
my own projects that consume the output of pandoc- and
skylighting-highlighted code blocks (by basically vendoring these
changes into those projects), so I am not personally blocked. I'm
contributing this change upstream in the hopes that it might help
unblock others.

[1]: https://jez.io/pandoc-markdown-css-theme/features/#numbered-and-highlighted-lines
[2]: edwardtufte/tufte-css#81 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants