Status: RESOLVED FIXED dev-doc-complete Core Components MathML (show other bugs) Trunk All All -- normal with 2 votes (vote) mozilla15 François Wang 572899 mathml-fonts 732830 782035 Show dependency tree / graph

Reported: 2012-03-15 01:56 PDT by Frédéric Wang (:fredw)
Modified: 2012-08-11 03:51 PDT (History)
7 users (show)
ryanvm: in‑testsuite?
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Attachments
Patch V1 (8.04 KB, patch)
2012-03-24 10:01 PDT, François Wang
karlt: review+
Details | Diff | Review
Patch V2 (8.05 KB, patch)
2012-03-26 09:00 PDT, François Wang
karlt: review+
Details | Diff | Review
Patch V4 (8.38 KB, patch)
2012-04-29 23:20 PDT, Frédéric Wang (:fredw)
karlt: review+
Details | Diff | Review

 Frédéric Wang (:fredw) 2012-03-15 01:56:57 PDT Followup of 663740 comment 8. Here is Karl's suggestion: >The use of gfxPlatform::ResolveFontName() is preventing the use of any mathfontNAME.properties files except for mathfontUnicode.properties with downloaded fonts. > Perhaps getting downloaded fonts working for stretchy characters is almost independent of that bug. The simplest solution there might be to simply stop using gfxPlatform::ResolveFontName() but instead have SetFontFamily in nsMathMLChar return a boolean to indicate success, returning success only when the nsFontMetrics is using the requested family or the nsGlyphTable is the Unicode table. > ResolvFontName doesn't know about the downloaded fonts. I > assume that the nsFontMetrics does but I haven't verified that. > > Changing the ResolveFontName call to a ResolverCallback would > leave detecting the presence of fonts up to the ResolverCallback > or TryVariants and TryParts. > > SetFontFamily callers that measure would have to check the return > code and behave appropriately. I expect that would be by > returning from the caller with the appropriate code. Callers that > draw can probably just ignore the return code. > > This is just the simplest modification I see to the existing code. > It won't be as efficient as ResolveFontName, but I expect there > are ways of improving efficiency if necessary. > > With ResolvFontName gone, I guess ResolverCallback should merge > into EnumCallback, or at least have a different name. > Frédéric Wang (:fredw) 2012-03-15 02:10:35 PDT This bug may also block bug 697053. I've written an add-on that adds WOFF math fonts: https://addons.mozilla.org/en-US/firefox/addon/mathml-fonts/ but it does not work yet with operators drawn using nsMathMLChar. Fixing this bug should make the add-on work as expected. François Wang 2012-03-24 10:01:45 PDT Created attachment 609012 [details] [diff] [review] Patch V1 Karl Tomlinson (ni?:karlt) 2012-03-25 17:00:44 PDT Comment on attachment 609012 [details] [diff] [review] Patch V1 Looks good, thanks. >+ aFont = font; Can you change this to "aFont.name = font.name" or "aFont.name = family", please? That will reduce how much needs to be copied. >+ if (!SetFontFamily(mChar->mStyleContext, mRenderingContext, >+ font, aGlyphTable, ch, aFamily)) return false; Please put the "return false" on a new line (indented, and leave the following blank line). That would make the jump statement more visible and is the more usual style for such a statement. François Wang 2012-03-26 09:00:34 PDT Created attachment 609342 [details] [diff] [review] Patch V2 Mozilla RelEng Bot 2012-03-28 13:26:17 PDT Autoland Patchset: Patches: 609342 Branch: mozilla-central => try Destination: http://hg.mozilla.org/try/pushloghtml?changeset=f8ae80f67d54 Try run started, revision f8ae80f67d54. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=f8ae80f67d54 Mozilla RelEng Bot 2012-03-28 18:17:19 PDT Try run for f8ae80f67d54 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=f8ae80f67d54 Results (out of 15 total builds): success: 15 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-f8ae80f67d54 Frédéric Wang (:fredw) 2012-03-29 12:27:56 PDT Another try, with the instruction to run the tests. Mozilla RelEng Bot 2012-03-29 12:31:36 PDT Autoland Patchset: Patches: 609342 Branch: mozilla-central => try Destination: http://hg.mozilla.org/try/pushloghtml?changeset=5ddb4b3e9982 Try run started, revision 5ddb4b3e9982. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=5ddb4b3e9982 Mozilla RelEng Bot 2012-03-29 18:47:01 PDT Try run for 5ddb4b3e9982 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=5ddb4b3e9982 Results (out of 284 total builds): success: 259 warnings: 24 failure: 1 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-5ddb4b3e9982 Frédéric Wang (:fredw) 2012-03-30 00:35:37 PDT Apparently, there are two failures on Windows: - layout/reftests/mathml/semantics-1.xhtml (similar to bug 572899) - layout/reftests/mathml/mfenced-10.html Ryan VanderMeulen [:RyanVM] 2012-03-30 18:00:24 PDT https://hg.mozilla.org/integration/mozilla-inbound/rev/f430bb8a0049 Ryan VanderMeulen [:RyanVM] 2012-03-30 20:48:45 PDT Yes indeed. Backed out. https://hg.mozilla.org/integration/mozilla-inbound/rev/031949d40753 (Feel free to take the checkin-needed flag off the bug next time you see Try failures...) Frédéric Wang (:fredw) 2012-03-31 10:41:35 PDT (In reply to Ryan VanderMeulen from comment #12) > (Feel free to take the checkin-needed flag off the bug next time you see Try > failures...) Sorry about that, I didn't notice that the checkin-needed flag was set. (In reply to Frédéric Wang from comment #10) > Apparently, there are two failures on Windows: > - layout/reftests/mathml/semantics-1.xhtml (similar to bug 572899) > - layout/reftests/mathml/mfenced-10.html It seems that mfenced-10 passes with this patch, so we can just remove the fails-if(winWidget) flag that was added in bug 553918. I hope that the patch from bug 572899 makes semantics-1 pass too. Ryan VanderMeulen [:RyanVM] 2012-03-31 10:45:11 PDT On the push I made, it was only passing on WinXP, FWIW. Frédéric Wang (:fredw) 2012-03-31 10:50:00 PDT (In reply to Ryan VanderMeulen from comment #14) > On the push I made, it was only passing on WinXP, FWIW. OK, you are right. We should modify fails-if(winWidget) by a weaker condition that excludes WinXP. Frédéric Wang (:fredw) 2012-04-01 03:02:53 PDT > OK, you are right. We should modify fails-if(winWidget) by a weaker > condition that excludes WinXP. I'm not sure whether there is a condition to do that. Maybe we can just mark the test random-if(winWidget)? Karl Tomlinson (ni?:karlt) 2012-04-01 21:39:14 PDT My suspicion re the reason for mfenced-10 failing is in bug 553918 comment 8, so in that respect the test is not really expected to pass and I'm happy for it to be marked as random on all platforms. But I don't understand why the changes in this bug should change the result of that test, so I wonder what we are missing. Frédéric Wang (:fredw) 2012-04-01 23:20:19 PDT In the screenshot of the test results, it seems that fences are built by parts, although the correct glyphs are not available. IIUC, we now always say that SetFontFamily succeeds when we use the Unicode table so I guess the unicode constructions are always tried, even when we don't have the basic fonts to do so. That does not really explain the relation with bug 553918 comment 8. Actually, I'm not really sure about why semantics-1 fails with this patch either. Karl Tomlinson (ni?:karlt) 2012-04-01 23:38:53 PDT (In reply to Frédéric Wang from comment #18) > In the screenshot of the test results, it seems that fences are built by > parts, although the correct glyphs are not available. That looks like a regression. > IIUC, we now always > say that SetFontFamily succeeds when we use the Unicode table so I guess the > unicode constructions are always tried, even when we don't have the basic > fonts to do so. That's making some sense. Previously the Unicode table was only tried when the fonts exist or the family was a generic family. That meant that the Unicode table wasn't used until after the "Symbol" table (on a typical WinXP machine). Emulating that logic with the new approach and the mTablesTried logic may be more difficult. Frédéric Wang (:fredw) 2012-04-26 02:40:03 PDT https://tbpl.mozilla.org/?tree=Try&rev=ca8fecde634f Frédéric Wang (:fredw) 2012-04-28 06:43:16 PDT Results look better with Karl's latest proposal: https://tbpl.mozilla.org/?tree=Try&rev=f7e1e1b02eec Mozilla RelEng Bot 2012-04-28 13:45:26 PDT Try run for f7e1e1b02eec is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=f7e1e1b02eec Results (out of 28 total builds): success: 26 warnings: 1 other: 1 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/fred.wang@free.fr-f7e1e1b02eec Timed out after 12 hours without completing. Frédéric Wang (:fredw) 2012-04-29 23:20:30 PDT Created attachment 619496 [details] [diff] [review] Patch V4 François is going to be offline in the next days, so I attach his latest patch. Ryan VanderMeulen [:RyanVM] 2012-04-30 17:32:39 PDT https://hg.mozilla.org/integration/mozilla-inbound/rev/a3563399583c Is it possible to create a test for this? :Ehsan Akhgari (busy, don't ask for review please) 2012-05-02 21:22:11 PDT https://hg.mozilla.org/mozilla-central/rev/a3563399583c Frédéric Wang (:fredw) 2012-05-19 00:13:16 PDT We should add a comment on dev release notes about this bug, mentioning that the MathML fonts add-on can now be used: https://addons.mozilla.org/en-US/firefox/addon/mathml-fonts/ (In reply to Ryan VanderMeulen from comment #24) > https://hg.mozilla.org/integration/mozilla-inbound/rev/a3563399583c > > Is it possible to create a test for this? A test is likely to be hard, because the rendering strongly depends on fonts available on the test machines. A test should also involve using @font-face rule that download a WOFF font placed somewhere on the test machine. Karl Tomlinson (ni?:karlt) 2012-05-20 14:59:21 PDT Fonts for reftests are typically included under layout/reftests/fonts. layout/reftests/font-face has several examples of their use. Note "HTTP(..)" in the reftest.list. There are already some DejaVu variants there that have support for stretchy operators. Perhaps a != test could check that using a different downloaded font changes the appearance of a stretchy operator. Florian Scholz [:fscholz] (MDN) 2012-06-06 11:50:08 PDT Frédéric has added notes here: https://developer.mozilla.org/en/Firefox_15_for_developers#MathML https://developer.mozilla.org/en/Mozilla_MathML_Project/Fonts#Main_fonts I consider this as documented then.

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