Bug 650530 (negative-namedspaces)

Names for negative spaces are not supported

RESOLVED FIXED in mozilla7

Status

()

Core
MathML
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: fredw, Assigned: Jonathan Hage)

Tracking

(Blocks: 2 bugs, {dev-doc-complete})

Trunk
mozilla7
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(2 attachments, 4 obsolete attachments)

(Reporter)

Description

6 years ago
We don't support the namedspaces negativeveryverythinmathspace, negativeverythinmathspace etc:
http://www.w3.org/TR/MathML/chapter2.html#type.length

This limitation is mentioned in MathJax's documentation as a non-available feature of Firefox:
http://www.mathjax.org/docs/1.1/output.html

The atoms are already declared here:
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsGkAtomList.h#1575

so I think the only piece of code to modify is:
http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLFrame.cpp#352
(Reporter)

Comment 1

6 years ago
Anyone working on this should be aware of bug 656429.
(Reporter)

Updated

6 years ago
Assignee: nobody → hage.jonathan
(Reporter)

Updated

6 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 2

6 years ago
Created attachment 539867 [details] [diff] [review]
Patch
Attachment #539867 - Flags: review?(karlt)
(Assignee)

Comment 3

6 years ago
Created attachment 539868 [details] [diff] [review]
Reftests
Attachment #539868 - Flags: review?(karlt)
(Reporter)

Comment 4

6 years ago
(In reply to comment #3)
> Created attachment 539868 [details] [diff] [review] [review]
> Reftests

Jonathan, it seems that you forgot to do "hg add" on your file
mathml-negativespace.html
(Assignee)

Comment 5

6 years ago
Created attachment 539879 [details] [diff] [review]
Reftests
Attachment #539868 - Attachment is obsolete: true
Attachment #539868 - Flags: review?(karlt)
Attachment #539879 - Flags: review?(karlt)
(Assignee)

Comment 6

6 years ago
Created attachment 539899 [details] [diff] [review]
Reftests
Attachment #539879 - Attachment is obsolete: true
Attachment #539879 - Flags: review?(karlt)
Attachment #539899 - Flags: review?(karlt)
Attachment #539899 - Flags: review?(karlt) → review+
Comment on attachment 539867 [details] [diff] [review]
Patch

Thanks!
Adding function names and more context makes it easier to review patches.  That can be done automatically with the [diff] block here:
https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3f

>-    if (aMathMLmstyleFrame) {
>+    if (aMathMLmstyleFrame) { 

Please remove the unnecessary whitespace change.
Whitespace changes can make it more difficult to analyse the history of a file, so we avoid them unless they correct the layout to match file style.
Attachment #539867 - Flags: review?(karlt) → review+
(Assignee)

Comment 8

6 years ago
Created attachment 540025 [details] [diff] [review]
Patch version 2
(Assignee)

Comment 9

6 years ago
Created attachment 540026 [details] [diff] [review]
Reftests version 2
(Assignee)

Comment 10

6 years ago
Comment on attachment 539867 [details] [diff] [review]
Patch

># HG changeset patch
># Parent bb3dcab53fc63d73890e28295d801f2cc6312da7
>
>diff --git a/layout/mathml/nsMathMLFrame.cpp b/layout/mathml/nsMathMLFrame.cpp
>--- a/layout/mathml/nsMathMLFrame.cpp
>+++ b/layout/mathml/nsMathMLFrame.cpp
>@@ -377,9 +377,37 @@
>     i = 7;
>     namedspaceAtom = nsGkAtoms::veryverythickmathspace_;
>   }
>+  else if (aString.EqualsLiteral("negativeveryverythinmathspace")) {
>+    i = -1;
>+    namedspaceAtom = nsGkAtoms::negativeveryverythinmathspace_;
>+  }
>+  else if (aString.EqualsLiteral("negativeverythinmathspace")) {
>+    i = -2;
>+    namedspaceAtom = nsGkAtoms::negativeverythinmathspace_;
>+  }
>+  else if (aString.EqualsLiteral("negativethinmathspace")) {
>+    i = -3;
>+    namedspaceAtom = nsGkAtoms::negativethinmathspace_;
>+  }
>+  else if (aString.EqualsLiteral("negativemediummathspace")) {
>+    i = -4;
>+    namedspaceAtom = nsGkAtoms::negativemediummathspace_;
>+  }
>+  else if (aString.EqualsLiteral("negativethickmathspace")) {
>+    i = -5;
>+    namedspaceAtom = nsGkAtoms::negativethickmathspace_;
>+  }
>+  else if (aString.EqualsLiteral("negativeverythickmathspace")) {
>+    i = -6;
>+    namedspaceAtom = nsGkAtoms::negativeverythickmathspace_;
>+  }
>+  else if (aString.EqualsLiteral("negativeveryverythickmathspace")) {
>+    i = -7;
>+    namedspaceAtom = nsGkAtoms::negativeveryverythickmathspace_;
>+  }
> 
>   if (0 != i) {
>-    if (aMathMLmstyleFrame) {
>+    if (aMathMLmstyleFrame) { 
>       // see if there is a <mstyle> that has overriden the default value
>       // GetAttribute() will recurse all the way up into the <mstyle> hierarchy
>       nsAutoString value;
Attachment #539867 - Attachment is obsolete: true
(Assignee)

Comment 11

6 years ago
Comment on attachment 539899 [details] [diff] [review]
Reftests

># HG changeset patch
># Parent bb3dcab53fc63d73890e28295d801f2cc6312da7
>
>diff --git a/layout/reftests/mathml/mathml-negativespace-ref.html b/layout/reftests/mathml/mathml-negativespace-ref.html
>new file mode 100644
>--- /dev/null
>+++ b/layout/reftests/mathml/mathml-negativespace-ref.html
>@@ -0,0 +1,54 @@
>+<html>	
>+  <head></head>
>+  <body>
>+    <p>
>+      <math>
>+	<mrow>
>+	  <mi>x</mi> <mspace width="negativeveryverythinmathspace"></mspace> <mi>y</mi> 
>+	</mrow>
>+      </math>
>+    </p>
>+    <p>
>+      <math>
>+	<mrow>
>+	  <mi>x</mi> <mspace width="negativeverythinmathspace"></mspace> <mi>y</mi> 
>+	</mrow>
>+      </math>
>+    </p>
>+    <p>
>+      <math>
>+	<mrow>
>+	  <mi>x</mi> <mspace width="negativethinmathspace"></mspace> <mi>y</mi> 
>+	</mrow>
>+      </math>
>+    </p>
>+    <p>
>+      <math>
>+	<mrow>
>+	  <mi>x</mi> <mspace width="negativemediummathspace"></mspace> <mi>y</mi> 
>+	</mrow>
>+      </math>
>+    </p>
>+    <p>
>+      <math>
>+	<mrow>
>+	  <mi>x</mi> <mspace width="negativethickmathspace"></mspace> <mi>y</mi> 
>+	</mrow>
>+      </math>
>+    </p>
>+    <p>
>+      <math>
>+	<mrow>
>+	  <mi>x</mi> <mspace width="negativeverythickmathspace"></mspace> <mi>y</mi> 
>+	</mrow>
>+      </math>
>+    </p>
>+    <p>
>+      <math>
>+	<mrow>
>+	  <mi>x</mi> <mspace width="negativeveryverythickmathspace"></mspace> <mi>y</mi> 
>+	</mrow>
>+      </math>
>+    </p>
>+  </body>	    
>+</html>
>diff --git a/layout/reftests/mathml/mathml-negativespace.html b/layout/reftests/mathml/mathml-negativespace.html
>new file mode 100644
>--- /dev/null
>+++ b/layout/reftests/mathml/mathml-negativespace.html
>@@ -0,0 +1,53 @@
>+<html>	
>+  <head></head>
>+  <body>
>+    <p>
>+      <math>
>+	<mrow>
>+	  <mi>x</mi> <mspace width="-0.0555555556em"></mspace> <mi>y</mi> 
>+	</mrow>
>+      </math>
>+    </p>
>+    <p>
>+      <math>
>+	<mrow>
>+	  <mi>x</mi> <mspace width="-0.111111111em"></mspace> <mi>y</mi> 
>+	</mrow>
>+      </math>
>+    <p>
>+      <math>
>+	<mrow>
>+	  <mi>x</mi> <mspace width="-0.166666667em"></mspace> <mi>y</mi> 
>+	</mrow>
>+      </math>
>+    </p>
>+    <p>
>+      <math>
>+	<mrow>
>+	  <mi>x</mi> <mspace width="-0.222222222em"></mspace> <mi>y</mi> 
>+	</mrow>
>+      </math>
>+    </p>
>+    <p>
>+      <math>
>+	<mrow>
>+	  <mi>x</mi> <mspace width="-0.277777778em"></mspace> <mi>y</mi> 
>+	</mrow>
>+      </math>
>+    </p>
>+    <p>
>+      <math>
>+	<mrow>
>+	  <mi>x</mi> <mspace width="-0.333333333em"></mspace> <mi>y</mi> 
>+	</mrow>
>+      </math>
>+    </p>
>+    <p>
>+      <math>
>+	<mrow>
>+	  <mi>x</mi> <mspace width="-0.388888889em"></mspace> <mi>y</mi> 
>+	</mrow>
>+      </math>
>+    </p>	
>+  </body>	    
>+</html>
>diff --git a/layout/reftests/mathml/reftest.list b/layout/reftests/mathml/reftest.list
>--- a/layout/reftests/mathml/reftest.list
>+++ b/layout/reftests/mathml/reftest.list
>@@ -60,3 +60,4 @@
> != scale-stretchy-4.xhtml scale-stretchy-4-ref.xhtml
> != scale-stretchy-5.xhtml scale-stretchy-5-ref.xhtml
> == math-as-mstyle-1.xhtml math-as-mstyle-1-ref.xhtml
>+== mathml-negativespace.html mathml-negativespace-ref.html
Attachment #539899 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Keywords: helpwanted → checkin-needed
(Reporter)

Comment 12

6 years ago
When this bug is fixed, I guess we should update the Mozilla doc to mention these attributes.

@Florian: Did you plan to put the MathML space names somewhere in the MathML reference doc? I was not able not find them in the <space> element reference and the attributes reference pages are empty.
Keywords: dev-doc-needed
(Reporter)

Comment 13

6 years ago
Landed on inbound.

http://hg.mozilla.org/integration/mozilla-inbound/rev/411e98c31217
http://hg.mozilla.org/integration/mozilla-inbound/rev/adfd866ec96c
Keywords: checkin-needed
Merged:
http://hg.mozilla.org/mozilla-central/rev/411e98c31217
http://hg.mozilla.org/mozilla-central/rev/adfd866ec96c
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
(Reporter)

Comment 15

6 years ago
> @Florian: Did you plan to put the MathML space names somewhere in the MathML
> reference doc? I was not able not find them in the <space> element reference
> and the attributes reference pages are empty.

From https://developer.mozilla.org/en/MathML/Attributes, I guess the answer is to put space names here:

https://developer.mozilla.org/en/MathML/Attributes/Values#Lengths
Hm, I thought I left a comment here. 
Any way, I started those two pages mentioned in comment 15.

Negative namedspaces are documented at [1] and mentioned on Firefox 7 for developers [2].

[1]https://developer.mozilla.org/en/MathML/Attributes/Values#Constants_%28namedspaces%29
[2]https://developer.mozilla.org/en/Firefox_7_for_developers#MathML

Marking as dev-doc-complete, but -as always- feel free to add, correct or review contents in the wiki.
Keywords: dev-doc-needed → dev-doc-complete
(Reporter)

Updated

6 years ago
Blocks: 687809
You need to log in before you can comment on or make changes to this bug.