Bug 114365 uses fontstyle and fontweight to implement bold/italic/bold-italic mathvariants due to concerns over people not having a font supporting the mathematical alphanumeric block. Once support of the mathematical alphanumeric block is sufficiently prevalent, the character transformation method used by the other mathvariant types should be adopted instead of fontstyle/fontweight.

The original function was designed for easy removal of the offending bits. Fixing the test suites was the tedious part.

Comment on attachment 8361652 [details] [diff] [review] fix.diff One change in this patch that warrants additional confirmation: mathvariant="bold" and font-weight="bold" no longer produce equivalent output. Is this fine?

Comment on attachment 8361652 [details] [diff] [review] fix.diff Review of attachment 8361652 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. Yes, the deprecated fontweight/fontstyle attributes should map to CSS font-weight/font-style while the mathvariant attribute should do the Unicode remapping, so they will render differently. We will need to wait that we support more MATH fonts before taking this patch.

Can this be checked in?

This is up to :fredw, but I think we are still waiting on improved math font support.

I think we will need at least 407059 and then the font-family update of 947654. Then we can ask users to switch to MATH fonts and this patch can be taken safely (and will be the right thing to do for MATH fonts).

This patch makes menclose-3-radical.html pass but subscript-italic-correction.html fail. https://tbpl.mozilla.org/?tree=Try&rev=e5962df46065 I suspect subscript-italic-correction.html could be a bit less strict. However: - on mobile devices, I think the problem is that no fonts with italic alphanum char is installed so they are just boxes without italic correction. - on Windows, I suspect Cambria Math is used and the handling of italic correction might be different (see bug 407059 comment 5 and bug 961482). James: can you check? I would be fine with marking subscript-italic-correction fail and fix that later. However, the missing glyphs on mobile devices is more a concern. On the other hand, without that patch the rendering looks of e.g. italic variable with new Open Type MATH fonts is "ugly". We make this patch applies conditionally with a #if macro, but that might not be convenient to handle the mathvariant tests differently.

So it seems that math fonts have different way to set the advance width & left/right bearings. In particular for STIX Math, Latin Modern Math and Cambria Math, the advance width seems to almost match the right bearing, so our computed italic correction is zero. Not sure what is the right way to do...

subscript-italic-correction.ref uses STIX-Regular for me, but it still fails due to the width/rightbearing issue of comment 9. This also has repercussions on non script uses of maths fonts. > <math> > <mtext mathvariant="italic" style="font-family:Cambria Math">office</mtext> > </math> Output shown in attachment. "office", "of fice" or "of f ice"? I've tested with Microsoft's equation editor and get an equivalent rendering, so they don't appear to apply any reverse italics correction. It is almost as if we need to override the metrics of specific glyphs. However that is probably a very nasty solution.

(In reply to James Kitchener (:jkitch) from comment #10) > I've tested with Microsoft's equation editor and get an equivalent > rendering, so they don't appear to apply any reverse italics correction. Did you try to write f_0 with the equation editor?

Per MATH table spec, italic correction is applied only “when a run of slanted characters is followed by a straight character (such as an operator or a delimiter)“ (and when positioning limits and super/subscripts of course).

Not until now. The subscript gets an "inverse italics correction" where appropriate.

I checked glyph info of the italic f (Cambria Math) with fontforge : - the advance width and right ink bound are almost the same (delta = 15) - the italic correction is only 60 so even if it would be reversed, it would not be enough to get the placement on attachment 8411836 [details]. - however the bottom right kernings are (for various heights) -400, -320 and 0. So that seems to be what ensure the correct placement of the subscript. So I think we need to fix bug 961482 to get the correct placement for Cambria Math.

Regarding the missing fonts, can’t we bundle a MATH font with the tests so that it works everywhere?

(In reply to Khaled Hosny from comment #15) > Regarding the missing fonts, can’t we bundle a MATH font with the tests so > that it works everywhere? We can use Web fonts to avoid missing fonts or override the behavior of Cambria Math ; or we could even mark the test as "fails". The main problem is not really to make the test pass but about what users will see in actual content (that is bad position of scripts with Cambria Math on Windows and more, seriously, empty boxes instead of variables on mobile devices). I think we should try to fix at least the missing fonts on mobile devices.

The changes to ensure math fonts on mobile devices might take some time and it's a shame that the rendering (especially italic variables) still does not work well with the new MATH fonts on desktop... So perhaps an alternative would be to take this change under an #ifdef, so that we only keep the old behaviors on mobile devices for now? (and mark the reftest failures appropriately)

What do you think about comment 17?

I'm OK with an ifdef. An alternative could be to test whether the requested characters have glyphs, but I don't know whether that would be worthwhile or not.

Let's see what's the #ifdef gives: https://tbpl.mozilla.org/?tree=Try&rev=e7534acc25fd

On second thoughts there are two other problematic platforms to consider Windows XP (without Office 2007+ installed) OSX10.6 Neither have maths fonts available by default. Both of those platforms are officially supported (at least in the immediate future) and an #ifdef won't help them.

(In reply to James Kitchener (:jkitch) from comment #21) > On second thoughts there are two other problematic platforms to consider > Windows XP (without Office 2007+ installed) > OSX10.6 > Neither have maths fonts available by default. However, we need a bit less than OpenType MATH fonts, here. We only need coverage for the Unicode Math Alpha num. OSX>=10.5 seems to have https://en.wikipedia.org/wiki/Apple_Symbols. I'm not sure for Windows XP, though (I've added a test mi-mathvariant-3 to check that)

reftest for OSX 10.6 failed, so I'm trying again with the Apple Symbols explicitly stated: https://tbpl.mozilla.org/?tree=Try&rev=754157d11a1b

it looks like it does not work on MacOS10.6 even with Apple Symbols listed and there does not seem to be any font for XP. So I'm trying another approach: https://tbpl.mozilla.org/?tree=Try&rev=2040bb10469d

Note that with Android and B2G, reftests are only run for Opt builds so "-p do" or two separate runs are required for complete coverage.

The previous one with only HasChar on the first font didn't seem to work: https://tbpl.mozilla.org/?tree=Try&rev=a25686d26b9e This one uses FindFontForChar to test more fonts, although I suspect it is less efficient.

So here is an updated patch with the result of the Try server. B2G, Android, /^Windows\x20NT\x205\.1/.test(http.oscpu) and OSX10.6 will still use the CSS style and so some mathvariant-* tests will fail. Also, OSX==10.8 does not seem to have the italic form for the dotless i, j (probably an issue with the STIXGeneral fonts?). subscript-italic-correction.html will fail with Cambria Math per discussions above. Karl, what do you think about that? Isn't it a perf problem to call FindFontForChar for each MathML letter in bold/italic/bold-italic?

(In reply to Frédéric Wang (:fredw) from comment #28) > Karl, what do you think about that? Isn't it a perf problem to call > FindFontForChar for each MathML letter in bold/italic/bold-italic? Perhaps a faster option would be to use gfxFontGroup::GetFirstMathFont() suggested by Jonathan on bug 961365 and call HasChar on it.

Comment on attachment 8422475 [details] [diff] [review] Patch (In reply to Frédéric Wang (:fredw) from comment #28) > Karl, what do you think about that? Isn't it a perf problem to call > FindFontForChar for each MathML letter in bold/italic/bold-italic? FindFontForChar() is essentially what shaping the TextRun will do (if it chooses to use these chars, so shouldn't be a perf problem, even without the unknown script optimization (see below). (In reply to Frédéric Wang (:fredw) from comment #30) > Perhaps a faster option would be to use gfxFontGroup::GetFirstMathFont() > suggested by Jonathan on bug 961365 and call HasChar on it. The gfx code is optimized for finding fonts for characters, including caches of character support. I don't know whether similar info is cached about whether the font has a math table, but I'd guess support is not as good. >+ uint8_t matchType = gfxTextRange::kFontGroup; No need to initialize matchType, as it is not used. (It is an outparam, that is sometimes not set if aPrevMatchedFont is non-null.) >+ FindFontForChar(ch2, 0, HB_SCRIPT_UNKNOWN, nullptr, &matchType); Note that some platforms will not search all fonts for HB_SCRIPT_UNKNOWN. [1] Author specified and some interpretation of the preferences for the generic (which I don't understand) will be searched. Perhaps that's what you want if you think that using style with one of the author-specified fonts would be better. If so, please add a comment indicating this. Otherwise, HB_SCRIPT_COMMON should be a thorough search, and I expect most of these characters would be script common. The performance difference should not be significant factor in the decision here. [1] http://hg.mozilla.org/mozilla-central/annotate/93e03b8c127e/gfx/thebes/gfxFont.cpp#l5516

Comment on attachment 8422928 [details] [diff] [review] Patch >- font-family: MathJax_Main, STIXGeneral, DejaVu Serif, DejaVu Sans, Cambria, Cambria Math, Times, Lucida Sans Unicode, OpenSymbol, Standard Symbols L, serif; >+ font-family: MathJax_Main, STIXGeneral, DejaVu Serif, DejaVu Sans, Cambria, Cambria Math, Times, Lucida Sans Unicode, OpenSymbol, Standard Symbols L, Apple Symbols, serif; Is there still a reason to include "Apple Symbols" here? If it doesn't help, then including an extra family will only slow down stretching.

(In reply to Karl Tomlinson (needinfo?:karlt) from comment #33) > Is there still a reason to include "Apple Symbols" here? > If it doesn't help, then including an extra family will only slow down > stretching. No, that didn't help for OSX10.6... I'll remove it.

Green try: https://tbpl.mozilla.org/?tree=Try&rev=0ee059cb7ec8

I'll try to submit this for Aurora, so that mathvariants work correctly with MATH fonts in Gecko 31 ESR.

Comment on attachment 8423118 [details] [diff] [review] Patch for Aurora [Approval Request Comment] Bug caused by (feature/regressing bug #): basic mathvariants in MathML rendering when modern math fonts are used. The bug has always been present but until Gecko 31, these new math fonts were not usable for MathML so that was not a concern (see below for details). User impact if declined: The basic mathvariants with the new math fonts will be displayed with a lower quality, especially for the ubiquitous case of italic math variables, so people won't be able/willing to use these new fonts and that will delay the transition (see below for details). Testing completed (on m-c, etc.): This change was tested on Try, Inbound and landed in mozilla-central last Thursday without problem. This change was already covered by existing MathML tests. A new mi-mathvariant-3.html has been added to ensure that the regression of attachment 8361652 [details] [diff] [review] does not happen with this new version (see below for details). Risk to taking this patch (and alternatives if risky): Low. This patch only touches MathMLTextRunFactory::RebuildTextRun and thus will only affect pages with MathML formulas. Only a few lines of code are added to check fonts that have Mathematical Alphanumeric Symbols and should not be a performance problem per comment 31. String or IDL/UUID changes made by this patch: None Details of why the patch is needed on aurora: Gecko 31 will have initial support for math fonts with an Open Type MATH table (https://developer.mozilla.org/en-US/Firefox/Releases/31#MathML). We want users to migrate to these modern Open Type fonts and to deprecate old fonts, so that we will be able to clean up our MathML/font code. It might take some time for this move to happen on all the platforms and so it seems a good idea to start making these modern fonts usable as soon as possible, and in particular in the ESR version. The basic mathvariants (italic, bold and bold italic) are currently handled via CSS style in Aurora. This works well for old fonts that are made of several font faces, but the Open Type MATH fonts only have a single "regular" font face and are normally designed to instead use the Mathematical Alphanumeric Symbols (http://en.wikipedia.org/wiki/Mathematical_Alphanumeric_Symbols). This means that the basic mathvariants with the new Open Type MATH fonts are currently displayed with a lower quality, especially for the ubiquitous case of italic math variables. Attachment 8361652 [details] [diff] was not taken before Gecko 31 moved to Aurora, because the default fonts on mobiles platforms and some old desktop platforms do not cover the Mathematical Alphanumeric Symbols, causing an even worse rendering with missing characters. The approach in this new patch is to use Mathematical Alphanumeric Symbols only when they are available, and to fallback to CSS style otherwise.

The approval-mozilla-aurora+ is enough to see the patch checked-in.