Skip to content

Conversation

@dpvc
Copy link
Member

@dpvc dpvc commented Nov 3, 2025

This PR fixes a number of problems with the data-latex attribute. Most notably, it restores missing initial characters in several situations, like numbers in subscripts (e.g., a_{123}), the initial motivation for this update. But it also adds missing backslashes in some cases (like a_\perp), missing macros like \begin in \begin{array}, and some other similar situations. Additionally, it removes unneeded spaces at the end of the attribute (but keeps them for \ ). Finally, it removes the data-latex-item attributes in the cleanAttributes filter.

All of this necessitated updates to the test files. Most of the changes are due to the removal of data-latex-item attributes, including of the missing initial characters, and the trimming of trailing spaces. There are a few substantive changes, mostly in the physics package, where things like \diffd end up as {\rm d} instead, so I hope that is OK. Because some of those macros go through several other macros, it is hard to decide which one they should show.

The most significant changes to the code are in TexParser.ts, in the updateResult() method, particularly the handling of super- and sub-scripts, which was the source of most of the problems, but there are also changes in a couple of other places.

The main issue was in the parse() method, where this.saveI is being set to this.i, but this.i is already past the character that initiated this parse method, so when that is used to get the substring used for data-latex, it is one character short. So I've subtracted a conditional 1 (subtract when a character other than &, since we don't want & as part of the data-latex for the cell, and some instances of the cases environment from mathtools would generate that). Right now, this is pretty ad hoc, and it probably needs a better mechanism where the handler (or maybe input) tells whether to subtract 1 or not, but for now this does the trick.

The other change to parse() is to not call updateResult() when the input is a macro. This was what was casing macros to be missing their backslashes in super- and subscripts, because the string would be just the macro name, and they would not be updated later when the backslash came through because the super- or subscript was not adjusted once it got a value. A macro first calls parse() for the backslash, which then calls parse() for the macro name, and since parse() calls updateResult() after handling the input, the macro calls updateResult() first, then the backslash calls it. So we will get the full macro name if we wait for the backslash to call updateResult() rather than calling it early with the macro name.

Given that, the main changes are in the updateResults() method. You'll probably want to look at this function as a whole rather than trying to read the diffs. I'v e added comments that I hope will tell you what is what, but the main problems were in how _ and ^ were handled. Rather than have the building of those latex strings depend on the currently saved latex being _ or ^ (which will only be the case once, but sometimes we need to adjust the latex several times), I have used a property to say which part is being processed. (That necessitated a change to NodeUtil.copyAttributes(), which was using setProperties() to copy the properties; but setProperties() sets attributes as well as properties, and that lead to properties becoming attributes unexpectedly.)

The mml() function is modified to only set the data-latex attribute if there actually is a value for it, and a trimTex() method is added that trims spaces from the beginning and end of a latex string, but retains the final space if it is needed for a \ macro. Finally, the composeLatex() method caches the value of TexConstant.Attr.LATEX so as to reduce the references to it.

The data-latex-item attributes are removed in the cleanAttributes() function in FilterUtil.ts, after they are no longer needed. To be honest, I'm not sure how they are actually being used, as they only seem to be used in updateResult() in obtaining existing, and since data-latex is set whenever data-latex-item is, I'm not sure why you can't use that instead. I suppose there are times when a node has data-latex without data-latex-item, so that might be it. I haven't tried removing it.

In ParseMethods.ts, I moved two lines so that they follow an early return that didn't need them.

I moved the addLatexItem() function from a static function in BaseItems.ts to a method in the StackItem class. That way, it can be called from other items, if needed, and can be overridden if need (I wanted to be able to do that while testing a patch for the poster of the issue, but couldn't).

Also in BaseItems.ts, lines 791-794 were collapsed since the ? are unneeded. (I think I made that change in a previous PR, but it snuck in here, too.)

Finally, in BaseMappings.ts, I combined some strings that used to be separated for line breaks, but prettier put them on one line, so no need to have them as two strings added. I also used substitution strings for one where that helped.

This resolves issue mathjax/MathJax#3459.

@dpvc dpvc requested a review from zorkow November 3, 2025 00:42
@dpvc dpvc added this to the v4.1.0 milestone Nov 3, 2025
@codecov
Copy link

codecov bot commented Nov 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.67%. Comparing base (27c47f3) to head (62bb93e).

Additional details and impacted files
@@            Coverage Diff             @@
##           develop    #1385     +/-   ##
==========================================
  Coverage    86.66%   86.67%             
==========================================
  Files          338      338             
  Lines        84384    84433     +49     
  Branches      4779     3153   -1626     
==========================================
+ Hits         73131    73180     +49     
  Misses       11253    11253             

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants