Last Comment Bug 320294 - (upright-mi) too much space between upright <mi> elements
(upright-mi)
: too much space between upright <mi> elements
Status: RESOLVED FIXED
[good second bug]
:
Product: Core
Classification: Components
Component: MathML (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla13
Assigned To: François Wang
:
Mentors:
Depends on:
Blocks: mathml-in-mathjax
  Show dependency treegraph
 
Reported: 2005-12-14 15:15 PST by David Harvey
Modified: 2014-09-13 13:24 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
illustrates spacing issues alluded to in comment #3 (4.28 KB, image/png)
2005-12-15 04:31 PST, David Harvey
no flags Details
mi spacing comparison (12.16 KB, image/png)
2012-03-07 09:09 PST, François Wang
no flags Details
mi spacing comparison with patch (11.74 KB, image/png)
2012-03-07 09:16 PST, François Wang
no flags Details
Patch V1 (1.47 KB, patch)
2012-03-07 09:20 PST, François Wang
karlt: review+
Details | Diff | Splinter Review

Description David Harvey 2005-12-14 15:15:33 PST
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/416.12 (KHTML, like Gecko) Safari/416.13
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

There is too much space between the "a" and "b" in the following kind of markup:

<mi fontstyle="normal">a</mi>
<mi fontstyle="normal">b</mi>

The space does not occur if one leaves out the font attributes altogether, i.e.

<mi>a</mi>
<mi>b</mi>

or if you use MathML version 2 font attributes, i.e.

<mi mathvariant="normal">a</mi>
<mi mathvariant="normal">b</mi>

This bug appears to be a partial relapse of comment 78 from bug 120198. That bug apparently got fixed for firefox 1.5, but what I'm describing here looks quite similar.


Reproducible: Always

Steps to Reproduce:
1. View a document containing the above MathML
2.
3.

Actual Results:  
Too much space between "a" and "b"

Expected Results:  
Essentially no space between "a" and "b". The behaviour should be identical to the situation where the "mathvariant" attribute is used instead.
Comment 1 David Harvey 2005-12-14 15:36:24 PST
And see also bug 306543 (which I couldn't quite find before....)
Comment 2 rbs 2005-12-14 22:02:39 PST
Seems to me that mathvariant should cause an inter-space. Don't we want a spacing at all between such tokens?!? Otherwise how does one distinguish with <mi>ab</mi>?
Comment 3 David Harvey 2005-12-15 04:31:57 PST
Created attachment 205949 [details]
illustrates spacing issues alluded to in comment #3
Comment 4 David Harvey 2005-12-15 04:32:50 PST
I disagree with rbs (comment 2). The problem is that if Firefox always puts a space between <mi> tokens, then there is *no* mechanism for an authoring tool to override this and force zero space. Sure, you can put them in the same <mi> block, but then what if you want to place two identifiers next to each other, which have different fonts? Or what if the second character is the base of a subscript? If the default space was zero, then the authoring tool always could use <mspace> to override this if it so desires.

Another way to put this is: TeX doesn't put any space between adjacent "identifiers" (in so far as that makes sense), and it looks *good* that way. See attachment (should have said "comment #4") for an example of what I mean. The first image is what I want it to look like. The second image is the best I could do with the current Firefox behaviour. (Image was produced with TeX.)
Comment 5 rbs 2005-12-15 15:49:48 PST
What about <mi>AA</mi> <mi>x</mi>? (Think <mi>sin</mi> <mi>x</mi>)

See the inter-spacing table here:
http://lxr.mozilla.org/mozilla/source/layout/mathml/base/src/nsMathMLContainerFrame.cpp#1186

It puts an inter-space between Upright and Italic and between Upright and Upright.

Should I revisit the table and disable all the inter-space, except when &af; is there? That is, treat "<mi>sin</mi> <mi>x</mi>" differently from "<mi>sin</mi> <mo>&af</mo> <mi>x</mi>". In other words, no space in the first, but inter-space in the second?
Comment 6 David Harvey 2005-12-15 16:15:21 PST
That sounds like a good solution to me. I guess that's what &ApplyFunction; is there for!
Comment 7 rbs 2005-12-15 20:48:08 PST
Be sure to think more about the consequences & be clear about the edge cases...

"<mi>T</mi> <mo>&af</mo> <mi>x</mi>"
"<mi fontstyle="normal">T</mi> <mo>&af</mo> <mi>x</mi>"
Comment 8 David Harvey 2005-12-16 04:55:37 PST
I would have thought the spacing in the two examples in comment 7 should be the same. The first is kind of like "T is a function which happens to be written in italics".

(Disclaimer: I am somewhat disinterested in the &af; case, since the code I am writing will never emit &af;.)
Comment 9 rbs 2005-12-16 17:18:22 PST
> the code I am writing will never emit &af;

How then do you intend to markup "\sin x"?
Comment 10 David Harvey 2005-12-16 19:48:03 PST
Using <mspace>. I am trying to write code which emulates TeX as closely as possible, and therefore needs fine control over spacing. Other converters, trying to achieve different objectives, may prefer to use &af;.
Comment 11 rbs 2005-12-16 21:40:17 PST
I am not a big fan of such an approach. This is the web, not pdftex... I like &af; better, or <mo lspace=".3em" rspace=".3em">sin</mo>, which is also recommended/accessible, and does what \mathop does in TeX, without being alienated.
Comment 12 David Harvey 2005-12-16 22:33:25 PST
I'm not the biggest fan either, but I don't want to get into a long discussion here, because that would force me to justify several major design decisions and several months of hard work :-) Let's just say that my code works out all the spacing according to TeX's rules, and consequently knows how much space there should be between "sin" and "x". The easiest thing to do, and what the code currently does, is to use <mspace>. Maybe I'll change it to use &af; more intelligently at some point.

But this seems to be a bit off-topic. The question is: will you make the changes you described above?
Comment 13 rbs 2005-12-17 00:22:04 PST
Possibly.

You can already collapse the inter-space with <mspace/>

      <mi fontstyle="normal">A</mi>
      <mspace width="0"/>
      <mi fontstyle="normal">x</mi>
      <mo>=</mo>
      <mi>&lambda;</mi><mspace width="0"/>
      <mi fontstyle="normal">x</mi>

See? I was warry of polluting the markup. I have less enticement if/when your are intentionally mspace-happy.
Comment 14 David Harvey 2005-12-17 00:34:28 PST
Thanks for pointing this out. I hadn't realised that <mspace width="0"/> would solve my problem.

So perhaps a truce :-) If you don't make any changes, then I'll make my code emit <mspace width="0"/> between any pairs of adjacent <mi> tokens (also <mn> tokens? <mtext>?), and we'll both be partially happy.

Related question: I notice that <mspace/> has the same effect as <mspace width="0"/> in this context. Can this behaviour be relied on?

I'm not sure exactly what you mean by "I have less enticement if/when your are intentionally mspace-happy."
Comment 15 rbs 2005-12-19 01:01:11 PST
I mean that if you are sooo happy to use mspace (which is already possible), then maybe I should concentrate more on the crashers :-) I have a number of them on my back right now and they have been earmarked by the drivers for 1.8.1 because of their potentially security-sensitive nature.
Comment 16 charan 2010-05-06 02:11:53 PDT
Again found this bug when executed Testcase id 54
Comment 17 charan 2010-05-11 00:13:21 PDT
Executed TC 45(TC document 4.5) & was able to reproduce
Comment 18 Frédéric Wang (:fredw) 2010-05-12 07:08:53 PDT
Which testcase are you talking about?
Comment 19 charan 2010-05-20 05:28:41 PDT
Executed TC 45 & was able to reproduce
Comment 20 Frédéric Wang (:fredw) 2011-06-07 12:29:56 PDT
There have recently been some reports from MathJax users of this issue. In MathJax, \mathrm{var} generates

<mi mathvariant="normal">v</mi>
<mi mathvariant="normal">a</mi>
<mi mathvariant="normal">r</mi>

and so is rendered "v a r" in Firefox, whereas the users would like to see only one variable "var". It does not seem possible to ask MathJax to force the generation of one <mi> since in other cases such that "2xy" we want to split into several tokens.

The MathML spec says:

"A typical graphical renderer would render an mi element as its content (See Section 3.2.1 MathML characters in token elements), with no extra spacing around it (except spacing associated with neighboring elements)."
Comment 21 Frédéric Wang (:fredw) 2012-02-23 01:09:58 PST
I think someone working on this will have to look to these parts of the code:

http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLTokenFrame.cpp#78

http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLContainerFrame.cpp#1094
Comment 22 Frédéric Wang (:fredw) 2012-03-04 11:54:10 PST
I had a look at this bug with François this week-end. Apparently, it's just a matter of modifying an entry in  kInterFrameSpacingTable.

I don't know if a reftest is necessary but I guess comparing

<mi mathvariant="normal">var</mi>

and

<mi mathvariant="normal">v</mi><mi mathvariant="normal">a</mi><mi mathvariant="normal">r</mi>

should work fine.
Comment 23 François Wang 2012-03-07 01:23:56 PST
It looks like eMathMLFrameType_UprightIdentifier is only used with the mi tag.
http://mxr.mozilla.org/mozilla-central/ident?i=eMathMLFrameType_UprightIdentifier

So it seems ok to change the value 0x01 in the spacing table as it is only used in the GetInterFrameSpacing:
http://mxr.mozilla.org/mozilla-central/ident?i=GET_INTERSPACE

So I changed the value (Upright, Upright) to 0x00, and now there is not too much space between the <mi> tags and mathvariant="normal". Without the mathvariant="normal" the frame type is eMathMLFrameType_ItalicIdentifier and in the spacing table the value of (Italic, Italic) is already set to 0x00.

Regarding reftests:

<mi mathvariant="normal">var</mi>

and

<mi mathvariant="normal">v</mi><mi mathvariant="normal">a</mi><mi mathvariant="normal">r</mi>

These two tests look different, in the second test after some letters (for instance "a" or "i"), there is a tiny space (and we don't see it unless we compare the two tests).
But actually, the same happens when frame type is Italic:

<mi>var</mi>

and

<mi>v</mi><mi>a</mi><mi>r</mi>

The second test is not rendered with the same width.

So we cannot use <mi>var</mi> as a reftest, but it is still possible to have the same render if we choose a string that only contains letters without the tiny space behind (for instance <mi>v</mi><mi>v</mi><mi>v</mi> should work)


I will attach the patch today (value 0x00 in the spacing table).
Comment 24 François Wang 2012-03-07 09:09:34 PST
Created attachment 603750 [details]
mi spacing comparison

This attachment shows the current spacing with mathvariant="normal". After some letters (f or i), the spacing is more important.
Comment 25 François Wang 2012-03-07 09:16:13 PST
Created attachment 603753 [details]
mi spacing comparison with patch

With the value 0x00 in the table, spacing is better with mathvariant="normal". But we can see a space after the letters a, f, i, k, l , m, r, t. And a space before the letter j.
Comment 26 François Wang 2012-03-07 09:20:51 PST
Created attachment 603754 [details] [diff] [review]
Patch V1
Comment 27 Karl Tomlinson (:karlt) 2012-03-07 14:46:23 PST
(In reply to François Wang from comment #23)
> <mi mathvariant="normal">var</mi>
> 
> and
> 
> <mi mathvariant="normal">v</mi><mi mathvariant="normal">a</mi><mi
> mathvariant="normal">r</mi>
> 
> These two tests look different, in the second test after some letters (for
> instance "a" or "i"), there is a tiny space (and we don't see it unless we
> compare the two tests).

The difference may be due to kerning only being applied when the character are in the same token.

But it is OK that these look different as they mean quite different things.
Comment 28 Frédéric Wang (:fredw) 2012-03-07 15:08:18 PST
(In reply to François Wang from comment #23)
> So we cannot use <mi>var</mi> as a reftest, but it is still possible to have
> the same render if we choose a string that only contains letters without the
> tiny space behind (for instance <mi>v</mi><mi>v</mi><mi>v</mi> should work)

(In reply to Karl Tomlinson (:karlt) from comment #27)
> The difference may be due to kerning only being applied when the character
> are in the same token.

So maybe it's not a good idea to choose letters without spaces because we don't know exactly on which parameters the kerning depends on and we may get a different results with another font for example.

What about comparing

<mi mathvariant="normal">v</mi>
<mi mathvariant="normal">a</mi>
<mi mathvariant="normal">r</mi>

with 

<mtext>v</mtext>
<mtext>a</mtext>
<mtext>r</mtext>

?
Comment 29 Karl Tomlinson (:karlt) 2012-03-07 15:40:53 PST
Only mi frames are eMathMLFrameType_UprightIdentifier, so mtext may be different.
Comment 30 Frédéric Wang (:fredw) 2012-03-07 22:14:04 PST
(In reply to Karl Tomlinson (:karlt) from comment #29)
> Only mi frames are eMathMLFrameType_UprightIdentifier, so mtext may be
> different.

OK, mtext frames are eMathMLFrameType_Ordinary. But in  kInterFrameSpacingTable, the spacing between Ord and Ord is also 0x00 so that may give the same result with the patch applied.

But do we really need a reftest for this bug?
Comment 31 Frédéric Wang (:fredw) 2012-03-07 22:58:00 PST
I pushed the patch to try server

https://tbpl.mozilla.org/?tree=Try&rev=ecef98f7d132

together with the two possibilities mentioned above for reftests:

https://hg.mozilla.org/try/rev/752136162710
Comment 32 Karl Tomlinson (:karlt) 2012-03-07 23:05:31 PST
I don't think we need to have a reftest.  This is an adjustment in aesthetic appearance, and I don't think we have any other mark-ups that should necessarily render exactly the same.
Comment 33 Frédéric Wang (:fredw) 2012-03-08 05:24:02 PST
(In reply to Karl Tomlinson (:karlt) from comment #32)
> I don't think we need to have a reftest.  This is an adjustment in aesthetic
> appearance, and I don't think we have any other mark-ups that should
> necessarily render exactly the same.

OK, anyway the suggestion in comment 28 works but not the one in comment 23 (there are failures on Windows and Android).

Otherwise, all looks good.
Comment 35 Marco Bonardo [::mak] 2012-03-09 05:10:15 PST
https://hg.mozilla.org/mozilla-central/rev/4574bdfa0ada

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