Closed
Bug 1108177
Opened 8 years ago
Closed 8 years ago
Mark positioning in Arabic broken in Firefox 34 with some fonts
Categories
(Core :: Graphics: Text, defect)
Tracking
()
RESOLVED
FIXED
mozilla38
People
(Reporter: mozilla, Assigned: jfkthame)
References
Details
Attachments
(4 files)
92.74 KB,
image/png
|
Details | |
13.65 KB,
patch
|
jtd
:
review+
|
Details | Diff | Splinter Review |
2.83 KB,
patch
|
jtd
:
review+
|
Details | Diff | Splinter Review |
185.62 KB,
image/png
|
Details |
Not sure what the characteristics are. Or if it's new in Firefox 34. But on my Linux box, this page is very broken: http://jsfiddle.net/ikopylov/0yeavgmk/ It works fine in Chrome.
Comment 1•8 years ago
|
||
No issue here, what version of Times New Roman do you have? I have 6.83 here.
Updated•8 years ago
|
Component: General → Graphics: Text
Product: Firefox → Core
Version: 17 Branch → 34 Branch
Reporter | ||
Comment 2•8 years ago
|
||
Humm. I'm looking at version 3.0 if I understand corrrectly. That said, HarfBuzz renders it correctly. My version has no GPOS, so the question is, if Firefox is indeed using the font I think it's using, why doesn't the interal HarfBuzz mark stacking not working for me.
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Behdad Esfahbod from comment #2) > Humm. I'm looking at version 3.0 if I understand corrrectly. That said, > HarfBuzz renders it correctly. My version has no GPOS, so the question is, > if Firefox is indeed using the font I think it's using, why doesn't the > interal HarfBuzz mark stacking not working for me. Could you post a screenshot to show the brokenness you're seeing, please?
Flags: needinfo?(mozilla)
Assignee | ||
Comment 5•8 years ago
|
||
The issue is that harfbuzz's fallback positioning doesn't work in Firefox, because we don't provide a get_glyph_extents callback. We should fix that.
Reporter | ||
Comment 6•8 years ago
|
||
Ah, good to know. Yes, please fix.
Assignee | ||
Comment 7•8 years ago
|
||
Having implemented basic loca/glyf-loading to support proper vertical metrics, we can refactor this slightly and also provide a glyph-extents callback, so that harfbuzz's fallback mark positioning works properly. This fixes the rendering of Behdad's example here, and similar cases with old (GPOS-less) fonts.
Attachment #8553023 -
Flags: review?(jdaggett)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Assignee | ||
Comment 8•8 years ago
|
||
And here's a simple reftest to check that it's working.
Attachment #8553028 -
Flags: review?(jdaggett)
Comment 9•8 years ago
|
||
Comment on attachment 8553023 [details] [diff] [review] Implement harfbuzz glyph-extents callback, so that fallback mark positioning works in legacy truetype fonts So this codepath only gets hit for marks from fonts lacking a GPOS table? So it's not really important to worry about how to deal with this issue for CFF fonts, the assumption being that there's a properly constructed GPOS table?
Reporter | ||
Comment 10•8 years ago
|
||
(In reply to John Daggett (:jtd) from comment #9) > Comment on attachment 8553023 [details] [diff] [review] > Implement harfbuzz glyph-extents callback, so that fallback mark positioning > works in legacy truetype fonts > > So this codepath only gets hit for marks from fonts lacking a GPOS table? So Correct. > it's not really important to worry about how to deal with this issue for CFF > fonts, the assumption being that there's a properly constructed GPOS table? We can even argue that CFF fonts must have GPOS. This is mostly for TrueType-but-not-OpenType fonts.
Comment 11•8 years ago
|
||
Comment on attachment 8553028 [details] [diff] [review] Reftest to check for fallback mark stacking >+++ b/layout/reftests/text/fallback-mark-stacking-1-notref.html >@@ -0,0 +1,23 @@ >+<body> >+These examples should NOT look the same: >+<div class=test>x̃̂ x̂̃</span> >+</body> This is kind of strange structure, was it intentional? Maybe </span> should be </div>?
Comment 12•8 years ago
|
||
From left to right, trunk with patch, Chrome 41.0.2272.3 dev, Safari 6.2.2, all running on OSX. Gentium - test font Fira Sans - otf version 3.002 Constantia - TrueType Microsoft cfont Candara - TrueType Microsoft cfont Minion Pro - otf, Adobe standard serif Cambria - TrueType Microsoft cfont Calibri - TrueType Microsoft cfont Fira Sans and Minion Pro lack support for U+302, U+303 but all the Microsoft cfont's do. Not sure why Constantia and Candara fail here.
Assignee | ||
Comment 13•8 years ago
|
||
(In reply to John Daggett (:jtd) from comment #11) > Comment on attachment 8553028 [details] [diff] [review] > Reftest to check for fallback mark stacking > > >+++ b/layout/reftests/text/fallback-mark-stacking-1-notref.html > >@@ -0,0 +1,23 @@ > > >+<body> > >+These examples should NOT look the same: > >+<div class=test>x̃̂ x̂̃</span> > >+</body> > > This is kind of strange structure, was it intentional? Maybe </span> should > be </div>? Oops. Yes, indeed.
Assignee | ||
Comment 14•8 years ago
|
||
(In reply to John Daggett (:jtd) from comment #12) > Created attachment 8554384 [details] > screenshot, patch vs. chrome vs. safari with different fonts > > From left to right, trunk with patch, Chrome 41.0.2272.3 dev, Safari 6.2.2, > all running on OSX. > > Gentium - test font > Fira Sans - otf version 3.002 > Constantia - TrueType Microsoft cfont > Candara - TrueType Microsoft cfont > Minion Pro - otf, Adobe standard serif > Cambria - TrueType Microsoft cfont > Calibri - TrueType Microsoft cfont > > Fira Sans and Minion Pro lack support for U+302, U+303 but all the Microsoft > cfont's do. Not sure why Constantia and Candara fail here. Because they have GPOS tables (and so the fallback positioning logic is not used) but don't include mark positioning features.
Comment 15•8 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #14) > Because they have GPOS tables (and so the fallback positioning logic is not > used) but don't include mark positioning features. Sort of unfortunate that we can deal with these fonts better. Safari displays marks correctly for both. But IE11 shows the same problem, so I guess we can deal with that later.
Comment 16•8 years ago
|
||
Comment on attachment 8553028 [details] [diff] [review] Reftest to check for fallback mark stacking Review of attachment 8553028 [details] [diff] [review]: ----------------------------------------------------------------- r+ with s/span/div/
Attachment #8553028 -
Flags: review?(jdaggett) → review+
Updated•8 years ago
|
Attachment #8553023 -
Flags: review?(jdaggett) → review+
Assignee | ||
Comment 17•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/02601ab9f9af https://hg.mozilla.org/integration/mozilla-inbound/rev/0edb544e9ebc
Target Milestone: --- → mozilla38
Comment 18•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/02601ab9f9af https://hg.mozilla.org/mozilla-central/rev/0edb544e9ebc
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•