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

Make sure explicit attributes added by \mmlToken are not removed. (mathjax/MathJax#2806) #757

Merged
merged 1 commit into from
Feb 22, 2022

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Jan 18, 2022

This PR modifies \mmlToken to make sure the attributes specified by it are not removed but the cleanAttributes filter, even if their values are the default values. This was causing the spacing for \bmod to be lost (and so it failed to have the proper space in super and subscripts).

Resolves mathjax/MathJax#2806.

@dpvc dpvc requested a review from zorkow January 18, 2022 13:36
@pkra
Copy link
Contributor

pkra commented Feb 3, 2022

I think this lets attributes bleed through to the output -- with this, I'm gettting underaccent="false" on g elements (from underset, I think).

@dpvc
Copy link
Member Author

dpvc commented Feb 3, 2022

OK, thanks, I'll check into it.

@pkra
Copy link
Contributor

pkra commented Feb 3, 2022

Sorry for not being more conclusive -- I'm testing issue2828, issue2806, issue2766, issue2764 and SRE4 final all at once. Lots of tiny visual changes but nothing too bad so far.

@dpvc
Copy link
Member Author

dpvc commented Feb 3, 2022

@pkra, no problem. We appreciate your reporting the issues.

@dpvc
Copy link
Member Author

dpvc commented Feb 3, 2022

OK, I found it. It is in #763, where I used underaccent instead of accentunder. OOPS! Because underaccent is not a MathML attribute, it is treated as one that you have added yourself, and so retains it in the output. I'll make a fix for the attributes. Thanks for the report, which will save me some more public embarrassment.

@pkra
Copy link
Contributor

pkra commented Feb 4, 2022

Ah, sorry for misreporting -- I should've started by just updating from develop.

@dpvc
Copy link
Member Author

dpvc commented Feb 4, 2022

No problem. I was pretty sure where the problem came from.

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.

@dpvc dpvc merged commit 4599d86 into develop Feb 22, 2022
@dpvc dpvc deleted the issue2806 branch February 22, 2022 13:53
@dpvc dpvc added this to the 3.2.1 milestone Feb 22, 2022
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.

3 participants