Persona is no longer an option for authentication on BMO. For more details see Persona Deprecated.
Bug 662756 - The default value for attributes lspace/rspace of <mo> should be thickmathspace
 Summary: The default value for attributes lspace/rspace of should be thickmathspace
 Status: RESOLVED FIXED dev-doc-complete Core Components MathML (show other bugs) Trunk All All -- normal (vote) mozilla16 Frédéric Wang (:fredw) Anthony Jones (:kentuckyfriedtakahe, :k17e) 557086 Show dependency tree / graph

Reported: 2011-06-08 02:01 PDT by Frédéric Wang (:fredw)
Modified: 2012-09-02 09:13 PDT (History)
4 users (show)
jaws: in‑testsuite+
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Attachments
Testcase (291 bytes, text/html)
2011-06-28 08:24 PDT, Jonathan Hage
no flags Details
Patch add thickmathspace (2.32 KB, patch)
2011-06-29 01:11 PDT, Jonathan Hage
no flags Details | Diff | Splinter Review
Patch V2 (3.15 KB, patch)
2012-06-05 11:46 PDT, Frédéric Wang (:fredw)
karlt: review-
Details | Diff | Splinter Review
Reftest (3.10 KB, patch)
2012-06-05 11:47 PDT, Frédéric Wang (:fredw)
karlt: review+
Details | Diff | Splinter Review
Patch V3 (2.20 KB, patch)
2012-06-22 07:46 PDT, Frédéric Wang (:fredw)
karlt: review+
Details | Diff | Splinter Review

 Frédéric Wang (:fredw) 2011-06-08 02:01:35 PDT The MathML REC says that the default values are "thickmathspace": http://www.w3.org/TR/MathML/chapter3.html#presm.mo.attrs In general, people use operators that are in our dictionary, so that's not a problem. Frédéric Wang (:fredw) 2011-06-08 02:04:38 PDT Here is an example in the MathML testsuite where we can see a problem: http://www.w3.org/Math/testsuite/build/main/Presentation/TokenElements/mo/mo6-full.xhtml Frédéric Wang (:fredw) 2011-06-21 13:23:23 PDT I'm guessing we only need to initialize lspace and rspace to the value of thickmathspace instead of 0 here: http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLmoFrame.cpp#386 Frédéric Wang (:fredw) 2011-06-21 22:27:35 PDT (In reply to comment #1) > Here is an example in the MathML testsuite where we can see a problem: > > http://www.w3.org/Math/testsuite/build/main/Presentation/TokenElements/mo/ > mo6-full.xhtml the same converted into reftest: https://github.com/fred-wang/MathJax-test/blob/master/MathMLToDisplay/Presentation/TokenElements/mo/mo6.html https://github.com/fred-wang/MathJax-test/blob/master/MathMLToDisplay/Presentation/TokenElements/mo/mo6-ref.html Jonathan Hage 2011-06-28 08:24:16 PDT Created attachment 542479 [details] Testcase Jonathan Hage 2011-06-29 01:11:55 PDT Created attachment 542738 [details] [diff] [review] Patch add thickmathspace Jonathan Hage 2011-06-29 04:14:44 PDT For both tests, we can see that the result is what we expected. But, with the tests proposed by Frédéric Wang, we can note there is a space not expected between f and (x). The applyfunction is treated like an mo that is not in the dictionary. If I comment the following condition: http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLmoFrame.cpp#182 the result is good. But there is a new problem, because the line is more below than the line of the reftest but I don't know why. To finish we can ask ourselves what we can do if the mo is not in the dictionary and at the beginning of the line. It would be better that the space shouldn't appear at the left of the mo. Frédéric Wang (:fredw) 2012-06-05 11:46:50 PDT Created attachment 630247 [details] [diff] [review] Patch V2 Frédéric Wang (:fredw) 2012-06-05 11:47:43 PDT Created attachment 630248 [details] [diff] [review] Reftest Frédéric Wang (:fredw) 2012-06-05 11:53:00 PDT https://tbpl.mozilla.org/?tree=Try&rev=9bfe662cd66a Karl Tomlinson (:karlt) 2012-06-05 22:06:06 PDT I assume that, if the scriptlevel and IS_ISOLATED tuning is appropriate for l/rspace from the dictionary, it would also be appropriate for default values. An implementation that changed the initialization of the lspace and rspace variables to the 5/18 default would be simpler and keep this tuning. Is there a reason why you wanted to avoid the tuning? Frédéric Wang (:fredw) 2012-06-05 22:57:09 PDT (In reply to Karl Tomlinson (:karlt) from comment #10) > An implementation that changed the initialization of the lspace and rspace > variables to the 5/18 default would be simpler and keep this tuning. > Is there a reason why you wanted to avoid the tuning? Yes, that's what I was wondering too. I don't have a strong reason, except trying to follow the MathML spec (but even thickmathspace = 5/18 is only a recommended value and I know that MathJax does not always follow the spacing recommended by the spec). I'll keep the tuning. (In general, I don't find this bug too important in practice, but it is a test in the MathML testsuite) Frédéric Wang (:fredw) 2012-06-07 04:10:27 PDT https://tbpl.mozilla.org/?tree=Try&rev=0a69b393ff4a Karl Tomlinson (:karlt) 2012-06-21 20:44:50 PDT Comment on attachment 630247 [details] [diff] [review] Patch V2 (Comment 10) Frédéric Wang (:fredw) 2012-06-22 07:46:46 PDT Created attachment 635743 [details] [diff] [review] Patch V3 I forgot to update my patch sorry. The previous Try Server results in comment 12 was for this new version. [Limited availability until Oct 31] Jared Wein [:jaws] (please needinfo? me) 2012-06-26 15:47:08 PDT Pushed to mozilla-inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/08f12e27bf51 https://hg.mozilla.org/integration/mozilla-inbound/rev/ef4be3efbb41 Ed Morley [:emorley] 2012-06-27 03:34:41 PDT https://hg.mozilla.org/mozilla-central/rev/08f12e27bf51 https://hg.mozilla.org/mozilla-central/rev/ef4be3efbb41 Mozilla RelEng Bot 2012-06-27 14:24:32 PDT Autoland Patchset: Patches: 630248, 635743 Branch: mozilla-central => try Patch 630248 could not be applied to mozilla-central. file layout/reftests/mathml/mo-lspace-rspace-ref.html already exists 1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/mathml/mo-lspace-rspace-ref.html.rej file layout/reftests/mathml/mo-lspace-rspace.html already exists 1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/mathml/mo-lspace-rspace.html.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir Patchset could not be applied and pushed. Florian Scholz [:fscholz] (MDN) 2012-09-02 09:13:13 PDT https://developer.mozilla.org/en-US/docs/Firefox_16_for_developers#MathML https://developer.mozilla.org/en-US/docs/MathML/Element/mo#Gecko-specific_notes

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