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

Work around WebKit bug with CHTML characters. (mathjax/MathJax#2435) #722

Merged
merged 2 commits into from
Jun 10, 2021

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Jun 9, 2021

This PR applies a CSS fix to the clipped character bug in Safari with CHTML. This is a long-standing bug, and it s great to have a fix for it. This was suggested by @mt4c in mathjax/MathJax#2435.

@dpvc dpvc added this to the 3.2 milestone Jun 9, 2021
@dpvc dpvc requested a review from zorkow June 9, 2021 17:17
@flacle
Copy link

flacle commented Jun 10, 2021

For the reviewer: issue still persists in Safari on both iOS 14.6 and Safari desktop version 14.1.1 after including the provided CSS snippet (see provided GIF). The issue seems to be caused by the chosen selector, which does not apply to newer versions of Safari (see provided screenshot taken from browserstrangeness). Would recommend to forward test instead with something like _::-webkit-full-page-media, _:future, :root .selector { property:value; } as this provides the most backwards compatibility. This one did the trick for me, but not sure how it performs with incremental versions.

  • GIF: mathjax_issue_722
  • Screenshot: browserstrangenness

@dpvc
Copy link
Member Author

dpvc commented Jun 10, 2021

@flacle, thanks for checking this out and reporting your results. I will adjust the PR. There is also new information from the original issue tracker, so I'm going to do a bit more experimentation first.

@dpvc
Copy link
Member Author

dpvc commented Jun 10, 2021

@flacle, I've update the PR. Can you test the new CSS rule, which is

    _::-webkit-full-page-media, _:future, :root mjx-container {
      will-change: opacity;
    }

Note that I've change from mjx-c::before to mjx-container to reduce the number of elements this applies to. It even works for me if I apply it to body, a single element, but I don't know what the consequences of doing that are, so feel safer applying to each mjx-container individually.

@flacle
Copy link

flacle commented Jun 10, 2021

@dpvc, no problem, will check it out. I also found this resource online https://solvit.io/bcf61b6 which claims of one selector that will work with any version (however browserstrangeness specifies that this works up until version 10) 🤷‍♂️

@dpvc
Copy link
Member Author

dpvc commented Jun 10, 2021

@flacle, Yes, I had seen that one as well, and it didn't work for my version, so I went with browserstrangeness. Thanks for checking it out.

@flacle
Copy link

flacle commented Jun 10, 2021

@dpvc, sure. I wanted to confirm that the updated suggestion works, no flickering when zooming in or out (pinch-to-zoom).

@dpvc
Copy link
Member Author

dpvc commented Jun 10, 2021

@flacle. Great! Thanks for the confirmation, and for being willing to help out.

@flacle
Copy link

flacle commented Jun 10, 2021

@dpvc no problem, glad I came across the issue just in time for a PR review.

Copy link
Member

@zorkow zorkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm!
As I am travelling I currently have not access to an IOS device or Safari.
The only webkit browser I got a hold of was Dolphin on Android, and while the original problem did not show up there was also no regression with the change.
Similarly, I did tests with a couple of browsers on Android, Linux and Win 10: No original problem and no regression.

So I fully defer to @flacle 's results!

@zorkow
Copy link
Member

zorkow commented Jun 10, 2021

@flacle Many thanks for doing the testing!

@mathjax mathjax deleted a comment from zorkow Jun 10, 2021
@dpvc dpvc merged commit 807a123 into develop Jun 10, 2021
@dpvc dpvc deleted the webkit-fix branch June 10, 2021 20:33
@flacle
Copy link

flacle commented Jun 11, 2021

@zorkow np! Looking forward to the new version on jsdelivr.net. Safe travels.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants