Closed Bug 1074223 Opened 10 years ago Closed 10 years ago

update OTS to pick up the fix from https://github.com/khaledhosny/ots/pull/35

Categories

(Core :: Layout: Text and Fonts, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35
Tracking Status
firefox34 --- fixed
firefox35 --- fixed

People

(Reporter: jfkthame, Assigned: jfkthame)

References

Details

Attachments

(2 files)

      No description provided.
If we also take the changes from https://github.com/khaledhosny/ots/pull/37, this will allow us to resolve bug 1074809.
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Blocks: 1074809
Attachment #8501939 - Flags: review?(jdaggett) → review+
I found another font that is erroneously rejected by OTS in the current Nightly 35.0a1 (2014-10-08) (GSUB table is discarded), but not by the upstream OTS version (https://github.com/khaledhosny/ots/commit/c24a839b1c66c4de09e58fabaacb82bf3bd692a4).

I believe that the font is valid. The GSUB table is valid when tested in MS Font Validator.

This is the console message for the equivalent WOFF file:

downloadable font: Layout: bad glyph count: 253 (font-family: "Web FontFont" style:normal weight:normal stretch:normal src index:0)
source: file:///Mister%20K/MisterKInformalWebPro_subset.woff MisterKInformalWebPro_subset.woff_proof.html:204
downloadable font: Layout: Failed to parse lookahead coverage table 0 in chain context format 3 (font-family: "Web FontFont" style:normal weight:normal stretch:normal src index:0)
source: file:///Mister%20K/MisterKInformalWebPro_subset.woff MisterKInformalWebPro_subset.woff_proof.html:204
downloadable font: Layout: Failed to parse chaining context format 3 subtable (font-family: "Web FontFont" style:normal weight:normal stretch:normal src index:0)
source: file:///Mister%20K/MisterKInformalWebPro_subset.woff MisterKInformalWebPro_subset.woff_proof.html:204
downloadable font: Layout: Failed to parse lookup subtable 5 (font-family: "Web FontFont" style:normal weight:normal stretch:normal src index:0)
source: file:///MisterKInformalWebPro_subset.woff MisterKInformalWebPro_subset.woff_proof.html:204
downloadable font: Layout: Failed to parse subtable 1 (font-family: "Web FontFont" style:normal weight:normal stretch:normal src index:0)
source: file:///Mister%20K/MisterKInformalWebPro_subset.woff MisterKInformalWebPro_subset.woff_proof.html:204
downloadable font: Layout: Failed to parse lookup 10 (font-family: "Web FontFont" style:normal weight:normal stretch:normal src index:0)
source: file:///Mister%20K/MisterKInformalWebPro_subset.woff MisterKInformalWebPro_subset.woff_proof.html:204
downloadable font: GSUB: Failed to parse lookup list table, table discarded (font-family: "Web FontFont" style:normal weight:normal stretch:normal src index:0)
source: file:///MisterKInformalWebPro_subset.woff
(In reply to Jens from comment #4)
> Created attachment 8502404 [details]
> MisterKInformalWebPro_subset.ttf
> 
> I found another font that is erroneously rejected by OTS in the current
> Nightly 35.0a1 (2014-10-08) (GSUB table is discarded), but not by the
> upstream OTS version
> (https://github.com/khaledhosny/ots/commit/
> c24a839b1c66c4de09e58fabaacb82bf3bd692a4).
> 
> I believe that the font is valid. The GSUB table is valid when tested in MS
> Font Validator.

I dumped the GSUB table from that font using TTX, and it looks to me like it is erroneous. An extract from Lookup index="10":

        <ChainContextSubst index="1" Format="3">
          <!-- BacktrackGlyphCount=0 -->
          <!-- InputGlyphCount=1 -->
          <InputCoverage index="0" Format="1">
            <Glyph value="d"/>
          </InputCoverage>
          <!-- LookAheadGlyphCount=1 -->
          <LookAheadCoverage index="0" Format="1">
            <Glyph value="H"/>
            <Glyph value="a"/>
            <Glyph value="a"/>
            <Glyph value="a"/>
            <Glyph value="d"/>
            <Glyph value="d"/>
            <Glyph value="d"/>
            <Glyph value="d"/>
            <Glyph value="e"/>
            <Glyph value="e"/>
            <Glyph value="e"/>
            <Glyph value="g"/>
            <Glyph value="g"/>
            <Glyph value="g"/>
            <Glyph value="l"/>
            <Glyph value="l"/>
            <Glyph value="l"/>
            <Glyph value="n"/>
            <Glyph value="n"/>
            <Glyph value="o"/>
            <Glyph value="o"/>
            <Glyph value="o"/>
            <Glyph value="s"/>
            <Glyph value="s"/>
            <Glyph value="s"/>
            <Glyph value="s"/>
            <Glyph value="v"/>
            <Glyph value="v"/>
            <Glyph value="v"/>
            <Glyph value="glyph00013"/>
            <Glyph value="glyph00013"/>
            <Glyph value="glyph00014"/>
            <Glyph value="glyph00015"/>

Note how there are lots of repeated glyphs in the list here. This should not be the case. Although the OpenType spec does not explicitly prohibit it (AFAICS), it makes a nonsense of the purpose of the Coverage table: "A Coverage table defines a unique index value (Coverage Index) for each covered glyph." If a glyph is listed repeatedly, it no longer has a unique index value!

So AFAICS the OTS messages quoted above are correct, and the font is faulty.
Whoa ... thanks for the tip. At first I thought that was an artifact of the subsetting that I did for this example font, but it turns out FontLab wrote the duplicate entries into the table.

I was able to fix the font with other tools. Thank you again.
(In reply to Jens from comment #6)
> Whoa ... thanks for the tip. At first I thought that was an artifact of the
> subsetting that I did for this example font, but it turns out FontLab wrote
> the duplicate entries into the table.

Oh, really? That sounds like a FontLab bug, then; I'd suggest you report it there.
https://hg.mozilla.org/mozilla-central/rev/5ec8e8f037bb
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 8501939 [details] [diff] [review]
Update OTS to pick up fixes for upstream issues 35, 37. Current rev: c24a839b1c66c4de09e58fabaacb82bf3bd692a4.

Approval Request Comment
[Feature/regressing bug #]:
The OTS update in bug 1057488 led to an unplanned change in warning-logging behavior that can result in spamming the developer console with messages.

[User impact if declined]:
Potential multi-second hang on some pages that use webfonts, see bug 1074809.

[Describe test coverage new/current, TBPL]:
Existing webfont tests exercise this code.

[Risks and why]:
Low risk, updating to new version of upstream library.

[String/UUID change made/needed]:
none
Attachment #8501939 - Flags: approval-mozilla-aurora?
I'd like to see this on m-c for a day or two before uplifting. If everything goes well on m-c, let's see if we can uplift on Sunday or, if not, before beta1.
Comment on attachment 8501939 [details] [diff] [review]
Update OTS to pick up fixes for upstream issues 35, 37. Current rev: c24a839b1c66c4de09e58fabaacb82bf3bd692a4.

Moving pending approval request from aurora to beta, as merge has happened in the meantime.
Attachment #8501939 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Comment on attachment 8501939 [details] [diff] [review]
Update OTS to pick up fixes for upstream issues 35, 37. Current rev: c24a839b1c66c4de09e58fabaacb82bf3bd692a4.

This has been on m-c for a bit now. Although a big change, I'm going to approve this for beta2 to address the multi-second hangs. Beta+
Attachment #8501939 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.