Closed
Bug 662756
Opened 13 years ago
Closed 12 years ago
The default value for attributes lspace/rspace of <mo> should be thickmathspace
Categories
(Core :: MathML, defect)
Core
MathML
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: fredw, Assigned: fredw)
References
()
Details
(Keywords: dev-doc-complete)
Attachments
(3 files, 2 obsolete files)
291 bytes,
text/html
|
Details | |
3.10 KB,
patch
|
karlt
:
review+
|
Details | Diff | Splinter Review |
2.20 KB,
patch
|
karlt
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•13 years ago
|
||
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
Blocks: 557086
Summary: mo should use use thickmathspace as a default value → The default value for attributes lspace/rspace of <mo> should be thickmathspace
Whiteboard: [good second bug]
Assignee | ||
Comment 2•13 years ago
|
||
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
Assignee | ||
Comment 3•13 years ago
|
||
(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
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → hage.jonathan
Comment 4•13 years ago
|
||
Comment 5•13 years ago
|
||
Comment 6•13 years ago
|
||
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.
Assignee | ||
Updated•13 years ago
|
Assignee: hage.jonathan → nobody
Assignee | ||
Updated•12 years ago
|
Whiteboard: [good second bug] → [mentor=fredw][lang=c++]
Assignee | ||
Updated•12 years ago
|
Attachment #542479 -
Attachment mime type: text/plain → text/html
Assignee | ||
Comment 7•12 years ago
|
||
Assignee: nobody → fred.wang
Attachment #542738 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #630247 -
Flags: review?(karlt)
Assignee | ||
Comment 8•12 years ago
|
||
Attachment #630248 -
Flags: review?(karlt)
Assignee | ||
Comment 9•12 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=9bfe662cd66a
Assignee | ||
Updated•12 years ago
|
Whiteboard: [mentor=fredw][lang=c++]
Updated•12 years ago
|
Attachment #630248 -
Flags: review?(karlt) → review+
Comment 10•12 years ago
|
||
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?
Assignee | ||
Comment 11•12 years ago
|
||
(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)
Assignee | ||
Comment 12•12 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=0a69b393ff4a
Comment 13•12 years ago
|
||
Comment on attachment 630247 [details] [diff] [review] Patch V2 (Comment 10)
Attachment #630247 -
Flags: review?(karlt) → review-
Assignee | ||
Comment 14•12 years ago
|
||
I forgot to update my patch sorry. The previous Try Server results in comment 12 was for this new version.
Attachment #630247 -
Attachment is obsolete: true
Attachment #635743 -
Flags: review?(karlt)
Updated•12 years ago
|
Attachment #635743 -
Flags: review?(karlt) → review+
Assignee | ||
Updated•12 years ago
|
Whiteboard: [autoland-try:-b do -p all -u all -t all]
Updated•12 years ago
|
Whiteboard: [autoland-try:-b do -p all -u all -t all] → [autoland-in-queue]
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Whiteboard: [autoland-in-queue]
Comment 15•12 years ago
|
||
Pushed to mozilla-inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/08f12e27bf51 https://hg.mozilla.org/integration/mozilla-inbound/rev/ef4be3efbb41
Comment 16•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/08f12e27bf51 https://hg.mozilla.org/mozilla-central/rev/ef4be3efbb41
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 17•12 years ago
|
||
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.
Assignee | ||
Updated•12 years ago
|
Keywords: dev-doc-needed
Comment 18•12 years ago
|
||
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
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•