KaTeX icon indicating copy to clipboard operation
KaTeX copied to clipboard

fix(auto-render): concatenate content of successive text nodes

Open marcoesters opened this issue 4 years ago • 4 comments

Webkit-based browsers like Chrome, Edge, or Safari do not parse very large math expressions (limit appears to be around 48k characters) because they split large text nodes into smaller ones so that auto-render misses delimiters. This PR fixes this by concatenating all successive text nodes.

Tested on:

  • Windows 10 with Chrome 96.0.4664.45, Edge 95.0.1020.53, and Firefox 93.0
  • MacOS 12.0.1 with Chrome 96.0.4664.55 and Safari 15.1
  • MacOS 10.13.6 with Safari 11.1.2
  • Android 12 with Chrome 95.0.4638.74 Firefox 94.1.2

What is the previous behavior before this PR?

Webkit-based browsers (Chrome, Edge, Safari) split large text nodes into smaller ones. This can cause delimiters to be in separate child nodes, which the auto-render parser will miss.

What is the new behavior after this PR?

All successive text modes will be concatenated into one node and the concatenated textContent will be used to render math.

marcoesters avatar Nov 19 '21 20:11 marcoesters

Codecov Report

Merging #3422 (d5ae92c) into main (99be728) will increase coverage by 0.02%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3422      +/-   ##
==========================================
+ Coverage   92.98%   93.00%   +0.02%     
==========================================
  Files          90       90              
  Lines        6739     6750      +11     
  Branches     1568     1570       +2     
==========================================
+ Hits         6266     6278      +12     
+ Misses        435      434       -1     
  Partials       38       38              
Impacted Files Coverage Δ
contrib/auto-render/auto-render.js 79.03% <100.00%> (+6.48%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 99be728...d5ae92c. Read the comment docs.

codecov[bot] avatar Nov 21 '21 19:11 codecov[bot]

Thanks -- this is looking good! Would you be able to add tests to contrib/auto-render/test/auto-render-spec.js? If not, I can try.

I'm working on a test with two strings long enough to break on Chrome (including and not including math to render). I'm not familiar enough with jest to know if it's possible to reproduce the issue in a unit test since the problem is browser-specific.

My tests pass with the current version of katex as well, so I could provide a much smaller string if a browser-specific test is not possible.

marcoesters avatar Nov 22 '21 21:11 marcoesters

I'm not sure what DOM the tests use, but it might be virtual. I had in mind tests where you manually make multiple adjacent text nodes, not necessarily large, and make sure the right substitutions happen (or don't happen).

edemaine avatar Nov 23 '21 04:11 edemaine

That makes a lot more sense than what I was trying to do.

The current tests should do the trick. The math parser fails in the current KaTeX version but passes with my mods.

marcoesters avatar Nov 23 '21 13:11 marcoesters

Thank you for the PR!

ylemkimon avatar Aug 29 '22 21:08 ylemkimon

:tada: This PR is included in version 0.16.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

KaTeX-bot avatar Aug 29 '22 21:08 KaTeX-bot