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

Two identical uses of \tag will cause identical element id's #240

Closed
dpvc opened this issue May 5, 2012 · 18 comments

Comments

Projects
None yet
3 participants
@dpvc
Copy link
Member

commented May 5, 2012

Since the \tag{xyz} macro produces a DOM element with id="mjx-eqn-xyz", two identical uses of \tag{} will cause two DOM elements to have identical id's, which is technically an error, since element id's are supposed to be unique.

Since MathJax uses the element id's to locate the elements associated with its internal MathML structure, that can cause confusion if the id's are not unique. See this post on meta.math.stackexchange.com for an instance where this occurred in the wild.

@dpvc

This comment has been minimized.

Copy link
Member Author

commented May 5, 2012

The math processing error listed in the link above can be avoided by a small change to the HTML-CSS output jax's HTMLcreateSpan() routine to have it check that the parent node is the correct one when reusing an existing span. But this doesn't avoid the duplicate id's and may cause problems with stretchy elements of they have an id that is already in use.

It may be possible to have the HTML-CSS output jax modify the id if it is already present, which will mean the equation is not exactly the same as the original input, but at least it will be properly displayed.

@fred-wang

This comment has been minimized.

Copy link
Contributor

commented May 19, 2012

I'm not sure how to write a test page that exhibit the [Math Processing Error]. So a reduced test case is welcome.

I've executed the LaTeXMathML tests with branch issue240 and no issues are found. I'll add a test to verify that id's are not duplicated.

@fred-wang

This comment has been minimized.

Copy link
Contributor

commented May 19, 2012

I'm trying to do

\(1 \tag{duplicateID}\)
\(2 \tag{duplicateID}\)

but both equations still get DOM id "mjx-eqn-duplicateID" on issue240 branch.

@fred-wang

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2012

OK, fd74ad5 only adds the parent node verification. Any suggestion for a reduced testcase?
The test suggested in my previous comment is not fixed yet, I'll add it when a fix is ready.

@fred-wang

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2012

Davide, can you please tell me whether you are done with this issue or you want to add more verifications for duplicate IDs. For the moment, I don't know how to test the parent node verification.

@dpvc

This comment has been minimized.

Copy link
Member Author

commented Aug 16, 2012

I'm still thinking about the duplicate ID issue. I suspect it can be resolved by having the output jax check for duplicate id's as is builds the output, and replacing the id for those with duplicates. But this will mean that user javascript or CSS that was wanting to refer to that element would no longer be able to. Of course, with two identical ID's they will still refer to something, so it is an error to have duplicate ID's anyway, so I think that would be OK.

I will also look for a test case for you when I get the chance.

@fred-wang

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2012

The duplicate ID issue is not fixed, so marking this Investigate.

@dpvc

This comment has been minimized.

Copy link
Member Author

commented Sep 6, 2012

I expect that that won't be handled in this release.

@fred-wang

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2012

OK, I'll mark this test fail. I'd still appreciate to have a testcase for the first issue before moving to ready for release.

@dpvc

This comment has been minimized.

Copy link
Member Author

commented Sep 6, 2012

If you use

\[x\tag{1}\]
\[\unicode{x2600}\tag{1}\]

this will introduce two identical ID's, and the \uncode reference will cause a file to be loaded (looking for data about that character), and that should trigger the processing error on the second equation. You could compare that to just the first equation followed by the actual HTML for the error message.

@fred-wang

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2012

Thanks for the testcase. I'm not sure what you mean for the reference file. But I guess I can just write a load test that will verify that the [Math Processing Error] does not happen when there is a duplicate id? It seems that it is what the issue240 branch fixes...

@fred-wang

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2012

LaTeXToMathML/references/tag-3.html
LaTeXToMathML/references/tag-4.html

@fred-wang

This comment has been minimized.

Copy link
Contributor

commented Sep 9, 2012

=> Ready For Release

@dpvc

This comment has been minimized.

Copy link
Member Author

commented Sep 9, 2012

I think these tests should do the trick. Yes, checking for the processing error is the key.

dpvc pushed a commit to dpvc/MathJax that referenced this issue Feb 15, 2014

@dpvc

This comment has been minimized.

Copy link
Member Author

commented Feb 15, 2014

I have added checks to make sure that an ID isn't reused. It checks the ID's from \tag{} and automatic numbering for equations typeset by MathJax, and other ID's already in the page. It doesn't include ID's applied by \cssID, however. When an ID is requested that is already in use, MathJax will find a unique one that is related to it (it appends a number). Of course, \label and ref will continue to work properly for the modified ID.

There need to be tests for more of the situations (the ID's differ when useLabelIds for example), and the connection between \ref and the labeled equations when duplicate ID's are used.

dpvc pushed a commit to dpvc/MathJax that referenced this issue Feb 17, 2014

@dpvc dpvc added Merged and removed Ready for Review labels Feb 17, 2014

@dpvc

This comment has been minimized.

Copy link
Member Author

commented Feb 17, 2014

=> Merged.

@dpvc dpvc closed this Feb 17, 2014

@dpvc dpvc added v2.4 and removed Merged labels Jun 30, 2014

@basvandertol

This comment has been minimized.

Copy link
Contributor

commented Sep 23, 2016

There is a bug in the fix.
The initialization of AMS.IDs and AMS.eqIDs is wrong.
The lines
AMS.eqlabels = AMS.eqIDs = {}; in TEX.prefilterHooks.Add
AMS.labels = AMS.IDs = {} in TEX.resetEquationNumbers
make the labels and IDs point to the same empty object.
This shared object then gets filled with both labels and IDs

@dpvc

This comment has been minimized.

Copy link
Member Author

commented Sep 23, 2016

Argh! Quite right.

dpvc added a commit that referenced this issue Sep 27, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.