324472 - Many mathml atoms seem unused

Status: RESOLVED WONTFIX

(Core :: MathML, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Many of the mathml atoms in nsGkAtomList.h (formerly nsMathMLAtomList.h) looks like they are unused. Examples are "abs", "apply", "arccos" and "xor".

Are there any reason these exist? The only advantage I can think of is to force these atoms to become permanent atoms and thus avoid refcounting or creating/deleting these atoms if we're going to handle them a lot.

If they are for parts of the spec not implemented yet, would it be ok to remove them for now and put them back when and if they're needed?

Comment 1

[Frédéric Wang (:fredw)]

These unused atoms belongs to "Content MathML" (bug 276028), but our implementation only supports "Presentation MathML".

https://developer.mozilla.org/en/Mozilla_MathML_Project/Status

I don't see any objections to remove them.

Comment 2

[Florian Scholz (Open Web Docs)]

Attachment: Removed unused "Content MathML" atoms V1

This patch removes unused atoms mostly belonging to "Content MathML". Some atoms for "Presentation MathML" are unused too, but I've kept them for now, as we plan to support those some day.

I will walk through the list in more detail again to spot forgotten atoms, but any feedback is appreciated as well.

Comment 3

[Florian Scholz (Open Web Docs)]

Attachment: Remove unused "Content MathML" atoms (V2)

Double-checked and corrected patch. Requesting review.

Comment 4

[Karl Tomlinson (:karlt)]

Comment on attachment 560543 [details] [diff] [review]
Remove unused "Content MathML" atoms (V2)

This would be changing the behaviour of nsTreeSanitizer, so asking Henri whether this would be OK.

BTW, the only mention I see of malign in the MathML 2 or 3 RECs is in the index and

'When notation has the value "longdiv", the contents are drawn enclosed by a
long division symbol. A complete example of long division is accomplished by
also using mtable and malign.'

http://www.w3.org/TR/MathML/chapter3.html#id.3.3.9.2

So maybe malign at least should be removed.

Comment 5

[Henri Sivonen (:hsivonen)]

Comment on attachment 560543 [details] [diff] [review]
Remove unused "Content MathML" atoms (V2)

The atom were used in the sanitizer quite deliberately in order not to corrupt Content MathML in clipboard operations.

Do we now have a good reason to want to reverse that decision and to corrupt Content MathML in clipboard operations? (FWIW, this bug was filed years before we started considering clipboard operations for MathML.)

Comment 6

[Karl Tomlinson (:karlt)]

Thanks, Henri.

I expect the cost of keeping these atoms is not so much that it would be a reason for choosing to corrupt Content MathML in clipboard operations.

Apologies to Florian that this wasn't noticed sooner.

I guess that makes this bug WONTFIX, unless there are some atoms that really are unused now, or we could recycle it for removing malign if that is the right thing to do.

Comment 7

[(no longer active)]

(In reply to Henri Sivonen (:hsivonen) from comment #5)
> Comment on attachment 560543 [details] [diff] [review]
> Remove unused "Content MathML" atoms (V2)
> 
> The atom were used in the sanitizer quite deliberately in order not to
> corrupt Content MathML in clipboard operations.
> 
> Do we now have a good reason to want to reverse that decision and to corrupt
> Content MathML in clipboard operations? (FWIW, this bug was filed years
> before we started considering clipboard operations for MathML.)

I don't see any reason we should do that!  I basically think that all of the changes to content/base/src/nsTreeSanitizer.cpp should be reverted here, and the atoms needed by that file should be added back.

Comment 8

[Frédéric Wang (:fredw)]

> I guess that makes this bug WONTFIX, unless there are some atoms that really
> are unused now, or we could recycle it for removing malign if that is the
> right thing to do.

David Carlisle confirmed on the W3C mailing list that malignmark is meant in the longdiv description. So I guess we can remove this malign atom.

Comment 9

[Florian Scholz (Open Web Docs)]

> I expect the cost of keeping these atoms is not so much that it would be a
> reason for choosing to corrupt Content MathML in clipboard operations.
Absolutely, let's keep them in that case.

I'd like to share some thoughts on the following atoms: 

[1] malign: see comment 8

[2] malignscope
"*Future* versions of MathML may provide an malignscope element that allows an alignment scope to be created around any MathML element [...]" (http://www.w3.org/TR/MathML3/chapter3.html#id.3.5.5.1) That is, malignscope isn't specified yet?

[3] mfraction
"[...] the non-MathML element mfraction (presumably in place of the MathML element mfrac) [...]" (http://www.w3.org/TR/MathML3/chapter3.html#id.3.3.5.3)
So "mfraction" does not exist and is just a sample for wrong markup ?

[4] monospaced 
The MathML3 spec index links to http://www.w3.org/TR/MathML3/chapter2.html#fund.attval but there is no "monospaced" element/attribute ?!?

[5] MathML constants (https://developer.mozilla.org/en/MathML/Attributes/Values) are neither attributes or elements, but removing them seems to be covered by bug 673759 already.

Comment 10

[Frédéric Wang (:fredw)]

(In reply to Florian Scholz [:fs] from comment #9)
> > I expect the cost of keeping these atoms is not so much that it would be a
> > reason for choosing to corrupt Content MathML in clipboard operations.
> Absolutely, let's keep them in that case.
> 
> I'd like to share some thoughts on the following atoms: 
> 
> [1] malign: see comment 8
> 
> [2] malignscope
> "*Future* versions of MathML may provide an malignscope element that allows
> an alignment scope to be created around any MathML element [...]"
> (http://www.w3.org/TR/MathML3/chapter3.html#id.3.5.5.1) That is, malignscope
> isn't specified yet?
> 
> [3] mfraction
> "[...] the non-MathML element mfraction (presumably in place of the MathML
> element mfrac) [...]" (http://www.w3.org/TR/MathML3/chapter3.html#id.3.3.5.3)
> So "mfraction" does not exist and is just a sample for wrong markup ?
> 
> [4] monospaced 
> The MathML3 spec index links to
> http://www.w3.org/TR/MathML3/chapter2.html#fund.attval but there is no
> "monospaced" element/attribute ?!?
> 
> [5] MathML constants
> (https://developer.mozilla.org/en/MathML/Attributes/Values) are neither
> attributes or elements, but removing them seems to be covered by bug 673759
> already.

Thanks Florian.
We should probably open a separate bug for [1-4], since this one is closed. [5] is already handled in bug 673759, as you noticed.