Last Comment Bug 650530 - (negative-namedspaces) Names for negative spaces are not supported
(negative-namedspaces)
: Names for negative spaces are not supported
Status: RESOLVED FIXED
[good first bug]
: dev-doc-complete
Product: Core
Classification: Components
Component: MathML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla7
Assigned To: Jonathan Hage
:
: Anthony Jones (:kentuckyfriedtakahe, :k17e)
Mentors:
Depends on:
Blocks: mathml-2 mathml-in-mathjax
  Show dependency treegraph
 
Reported: 2011-04-16 12:19 PDT by Frédéric Wang (:fredw)
Modified: 2011-09-20 04:01 PDT (History)
3 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (1.57 KB, patch)
2011-06-16 12:19 PDT, Jonathan Hage
karlt: review+
Details | Diff | Splinter Review
Reftests (1.76 KB, patch)
2011-06-16 12:21 PDT, Jonathan Hage
no flags Details | Diff | Splinter Review
Reftests (1.76 KB, patch)
2011-06-16 13:19 PDT, Jonathan Hage
no flags Details | Diff | Splinter Review
Reftests (2.95 KB, patch)
2011-06-16 14:35 PDT, Jonathan Hage
karlt: review+
Details | Diff | Splinter Review
Patch version 2 (1.95 KB, patch)
2011-06-17 04:06 PDT, Jonathan Hage
no flags Details | Diff | Splinter Review
Reftests version 2 (3.02 KB, patch)
2011-06-17 04:08 PDT, Jonathan Hage
no flags Details | Diff | Splinter Review

Description Frédéric Wang (:fredw) 2011-04-16 12:19:30 PDT
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
Comment 1 Frédéric Wang (:fredw) 2011-05-11 13:08:55 PDT
Anyone working on this should be aware of bug 656429.
Comment 2 Jonathan Hage 2011-06-16 12:19:31 PDT
Created attachment 539867 [details] [diff] [review]
Patch
Comment 3 Jonathan Hage 2011-06-16 12:21:22 PDT
Created attachment 539868 [details] [diff] [review]
Reftests
Comment 4 Frédéric Wang (:fredw) 2011-06-16 12:23:37 PDT
(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
Comment 5 Jonathan Hage 2011-06-16 13:19:00 PDT
Created attachment 539879 [details] [diff] [review]
Reftests
Comment 6 Jonathan Hage 2011-06-16 14:35:10 PDT
Created attachment 539899 [details] [diff] [review]
Reftests
Comment 7 Karl Tomlinson (:karlt) 2011-06-17 01:15:06 PDT
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.
Comment 8 Jonathan Hage 2011-06-17 04:06:48 PDT
Created attachment 540025 [details] [diff] [review]
Patch version 2
Comment 9 Jonathan Hage 2011-06-17 04:08:20 PDT
Created attachment 540026 [details] [diff] [review]
Reftests version 2
Comment 10 Jonathan Hage 2011-06-17 04:11:22 PDT
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;
Comment 11 Jonathan Hage 2011-06-17 04:11:53 PDT
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
Comment 12 Frédéric Wang (:fredw) 2011-06-19 02:09:23 PDT
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.
Comment 15 Frédéric Wang (:fredw) 2011-06-25 07:36:24 PDT
> @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
Comment 16 Florian Scholz [:fscholz] (MDN) 2011-06-25 09:50:35 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.