Create preference to disable MathML
We should have a way to disable MathML support in Firefox for the security slider. There currently is no pref in Firefox for this, so we will need to create one.
Activity
Kathy and I spent some time looking at this over the past couple of days. We have not assigned the bug to one of us yet because we have not gotten very far. There are a couple of approaches we are looking at:
- Remove knowledge of and related tags from the parser. This will work with limitations, but it may be difficult to access prefs. deep inside the parser (e.g., the code is maintained in Java and translated to C++). Also, the parser is not invoked when JS is used to add elements, so we would need a different solution for that case.
- Modify NS_NewElement() to treat and related tags as unknown/generic tags instead of MathML tags. This seems like a good solution, but so far we have not been able to get it to work (other code still recognizes the elements as MathML).
- Disable special behavior / rendering / etc. for MathML tags. We have not tried this yet, but it might be do-able via modifications to content/mathml/content/src/nsMathMLElement.cpp. We will continue to work on this; if anyone else has an idea for how to cleanly do this kind of thing inside Firefox, please comment here.
-
Replying to mcs:
- Modify NS_NewElement() to treat and related tags as unknown/generic tags instead of MathML tags. This seems like a good solution, but so far we have not been able to get it to work (other code still recognizes the elements as MathML).
A quick update: we got the above approach to work. The trick we are using is to replace the namespace on and related elements with the generic XML namespace. We will tie this to a pref. now.
-
Our proposed fix is here: https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug13548-01&id=5b48b325cf54fe341c9c6c417ecfbb83cc5625be (on the bug13548-01 branch within the user/brade/tor-browser.git repo).
Please review for code correctness as well as approach.
Trac:
Status: assigned to needs_review
Keywords: N/A deleted, TorBrowserTeam201503R, firefox-patch added -
I already found a disadvantage of this approach: some pages have CSS that tries to make up for a lack of MathML support, but the CSS will not work if it relies on the MathML XML namespace (which our patch conditionally removes). See for example: https://developer.mozilla.org/en-US/docs/Web/MathML/Examples/Deriving_the_Quadratic_Formula
I am not sure if this is a fatal flaw or not, but it is disappointing.
Replying to gk:
What speaks against the "just don't render these tags" (should we make a non-chrome/chrome distinction?), directly? That would give us what we want while not breaking the CSS "fallback". It is option three from above if I understood it correctly, I think.
Nothing speaks against it except we did not figure out how to do that yet. But we will take a look.
-
It turns out that approach 3 (disable special behavior / rendering / etc. for MathML tags) is messy. A lot of special behavior is based on the XML namespace, so removing that (as the existing patch does) made a lot of things simpler. Kathy and I are still looking, but fort approach 3 so far we have:
- In the following two files, we would need to ensure that the mathml.css stylesheet is not added to the document:
- content/mathml/content/src/nsMathMLElement.cpp
- parser/htmlparser/src/nsExpatDriver.cpp
- In layout/base/nsCSSFrameConstructor.cpp we would need to avoid calling FindMathMLData(). We would also need to add "if (prefIsDisabled)" checks in about 6 other places where special behavior is invoked in response to the presence of a MathML element (look for kNameSpaceID_MathML in that file).
- In browser/base/content/nsContextMenu.js, we would need to make changes to ensure that the "View MathML Source" context menu item is not shown when MathML is disabled.
- We may need to make changes to content/base/src/nsTreeSanitizer.cpp, although it is probably OK to allow the math tags through even if MathML is disabled. I am not sure exactly when nsTreeSanitizer is used; maybe only when displaying RSS/Atom feeds.
In summary, a lot of changes are required, which will lead to less confidence that we have truly disabled all of the MathML functionality (as well as a somewhat complex patch to maintain).
Opinions? The "swap the namespace" patch that we already posted is a lot simpler and we are confident that it disables MathML. On the other hand, because the MathML namespace is lost, it breaks some assumptions made by web pages.
For the record, Kathy is firmly in the "let's go with the existing namespace patch" and I am undecided. But I too do not like the extra complexity of approach 3, and MathML is probably uncommon enough that no one will complain about the namespace issue.
Replying to mcs:
Opinions? The "swap the namespace" patch that we already posted is a lot simpler and we are confident that it disables MathML. On the other hand, because the MathML namespace is lost, it breaks some assumptions made by web pages.
For the record, Kathy is firmly in the "let's go with the existing namespace patch" and I am undecided. But I too do not like the extra complexity of approach 3, and MathML is probably uncommon enough that no one will complain about the namespace issue.
Let's try the patch you already have then and see what breaks. Especially given the time constraints we have for the release and keeping in mind that there is #12827 (moved) waiting as well. Putting it back to needs_review again.
Trac:
Status: needs_information to needs_review
Keywords: N/A deleted, TorBrowserTeam201503R added-
Replying to gk:
Let's try the patch you already have then and see what breaks. Especially given the time constraints we have for the release and keeping in mind that there is #12827 (moved) waiting as well. Putting it back to needs_review again.
I think taking the current patch and seeing what breaks is a good approach. I am open to revisiting the issue after Firefox 38.
