Repository navigation
No innerHTML and pseudo-html cleanup - #1792
Merged
Merged
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Started out as a bugfix but kind of turned into a spring cleaning...
In an effort to fix bounding box calculations in IE (one part of #1782) - which were broken by trying to use the
innerHTMLproperty of an SVG element - I wound up going through every user ofDrawing.bBoxand trying to make sure it was a<text>or.*-math-groupnode that had previously hadsvgTextUtils.convertToTspanscalled on it. Because if so, I could use thedata-unformattedattribute, along with whether or not the node was converted to MathJax, in place of theinnerHTMLin the cache key, which is also much smaller than theinnerHTML, reducing my memory concerns about this cache.But then while doing THAT it became clear that all the monkey business using
tspan.lineelements to count lines and to properly align multi-line text was a disaster, and not done properly in a lot of places, so I centralized all that in two new utilities:svgTextUtils.positionText- to be used instead ofDrawing.setPosition, handles multiline alignment, and also gets called withinconvertToTspansso if you usetransforminstead of thexandyattributes to position the text, you never even need to use this at all.svgTextUtils.lineCount- nuff said. No more hacky and/or broken selectors for counting lines of text.So after that, almost nobody outside of
svgTextUtilseven needs to know about<tspan>or the.lineclass.Along the way I found a few places we were not supporting or had bugs supporting pseudo-HTML but easily could, and added it: rangeselectors, updatemenus, sliders, carpet axes (titles and tick labels). And in cases that currently break with MathJax I explicitly cancel it (with the
data-notexattribute). (MathJax test mocks coming in a future PR)And while I was working in
test_syntax.jsI took care of #1773 as well.There are still a few
Drawing.bBoxusers that call it on elements larger than a<text>or MathJax group node, that at this point in this branch we will NOT cache at all. For 🐎 I need to either convert them to their base nodes or find an alternate way to give them a robust cache key: