633322 - "ASSERTION: detailed glyph record missing!" uppercasing ß, followed by zwsp

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: detailed glyph record missing!: 'mLastUsed != nsTArray<DGRec>::NoIndex', file gfxFont.h, line 2105

###!!! ASSERTION: invalid array index: 'i < Length()', file nsTArray.h, line 455

Seems to be a regression from bug 631035.  Security-sensitive because it's using nsTArray's ElementAt (not SafeElementAt).

It seems to be important that the second character is ß (which uppercases to SS) and the third character is non-rendered (U+200B ZWSP or U+2061 FUNCTION APPLICATION).  I don't know what the deal with the first character is; on my machine it's a hexbox.

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Jonathan Kew [:jfkthame]]

Yes, this is certainly a regression from bug 631035; I'll dig into it ASAP. Thanks for the testcase!

This probably accounts for the crashes reported in bug 633453.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: patch, ensure GetDetailedGlyphs is not called when glyph count is zero

The DetailedGlyphStore is not intended to be called for character indexes that don't have any detailed glyphs; callers are expected to check this before trying to retrieve the DetailedGlyphs pointer. I missed one in MergeCharactersInTextRun().

So the real change here is just to check GetGlyphCount() in MergeCharactersInTextRun(). I've checked that all other current callers of GetDetailedGlyphs look correct, but I've also added extra comments and assertion checks at GetDetailedGlyphs, to help us catch any future issues quickly.

We should add this example to crashtests, too.

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: patch, add the testcase to gfx/tests/crashtests

Comment 5

[Jonathan Kew [:jfkthame]]

Moving this to Graphics, as it's a gfx bug rather than layout.

(In reply to comment #2)
> This probably accounts for the crashes reported in bug 633453.
FTR, that turned out to be a separate error, though they're both regressions from 631035.

Comment 6

[Jonathan Kew [:jfkthame]]

http://hg.mozilla.org/mozilla-central/rev/fcf6c9b3bd7d (patch)
http://hg.mozilla.org/mozilla-central/rev/f36e81d4d60d (crashtest)