STEPS TO REPRODUCE 1. load URL 2. Choose UTF-16 from the View->Character Encoding menu Possibly related bug on Windows on the same URL: bug 375773 ACTUAL RESULT Crash, see attached stack. Assertions leading up to the crash: ###!!! ASSERTION: First character must be the start of a cluster and can't be a ligature continuation!: 'aCharIndex > 0 || (aGlyph.IsClusterStart() && !aGlyph.IsLigatureContinuation())', file ../../../dist/include/thebes/gfxFont.h, line 888 ###!!! ASSERTION: Ligature at the start of the run??: 'ligStart > 0', file gfxFont.cpp, line 605 PLATFORMS AND BUILDS TESTED Bug occurs in trunk debug build (updated today)

Worth attaching the page to the bug in case it changes...

reproducible with data:text/html,‮쎭‼&#x202c

Problem is that gfxPangoFontGroup::SetGlyphs gets PangoGlyphString glyphs in a different order to what it expects, and then gets all confused at: } else if (glyphCount == numGlyphs || PRUint32(logClusters[glyphIndex]) > index) { // !!! // No glyphs for this cluster, and it's not a null byte. if (!aTextRun->GetCharacterGlyphs()[utf16Offset].IsClusterContinuation()) { // It must be part of a ligature. aTextRun->SetCharacterGlyph(utf16Offset, g.SetLigatureContinuation()); } Will see if I can make this more robust.

The order of the glyphs returned from pango_shape depends on the shape engine used for the language. If the language is en or el, glyphs for RTL text appear to be in reverse-logical order. If the language is ko, glyphs for RTL text appear to be in logical order. This is contrary to the documentation of PangoGlyphVisAttr in pango 1.16.2: "Clusters are stored in visual order, within the cluster, glyphs are always ordered in logical order, since visual order is meaningless; that is, in Arabic text, accent glyphs follow the glyphs for the base character." The swap_range() functions used for RTL text by pango_ot_buffer_output() in pango-1.16.2/pango/pango-ot-buffer.c and fallback_shape() in pango-1.16.2/modules/basic/basic-fc.c, swap all glyphs irrespective of cluster position, so we cannot expect glyphs within clusters are to be in logical order. No glyph swapping is performed in the Hangul shape engine (pango-1.16.2/modules/hangul/hangul-fc.c) for any text direction so we cannot expect clusters to be in "visual" order. It seems that the most we can assume about the order of the glyphs returned from pango_shape is that glyphs within a cluster are grouped together, and perhaps that the orderings are consistent within each PangoGlyphString (because there is one text direction and one shape engine). PangoGlyphInfo.attr.is_cluster_start is set in the top level of pango_shape (pango-1.16.2/pango/shape.c) based on the log_clusters information, so this should be consistent across shape engines, marking the first glyph in the cluster when glyphs are read in PangoGlyphString order.

(In reply to comment #4) > reproducible with data:text/html,‮쎭‼&#x202c The above testcase won't crash when CreateGlyphRunsFast is removed in attachment 263598 [details] [diff] [review] of bug 357637. But expect problems with data:text/html,‮쎭쉎

With the checkin of attachment 263898 [details] [diff] [review] in bug 357637, * the testcase in comment 4 now doesn't crash. * the original URL with UTF-16 now doesn't crash (possibly because I don't have fonts for all characters installed). * the testcase in comment 7 now only crashes if the (Korean) fonts are installed.

Always process glyph clusters in logical order. This resolves the remaining crash case, and also fixes bug 380045. Will do some more testing.

Comment on attachment 264434 [details] [diff] [review] alpha patch alpha patch has tested fine.

+ } while (logClusters[glyphIndex] == (gint)clusterUTF8Start && + glyphIndex < numGlyphs); Need to reverse the order of these tests. Use constructor-style casts: gint(...) instead of (gint)... + } while(nextGlyphClusterStart < 0 && aUTF8[utf8Index] != '\0'); missing space between while and ( Basically it looks great.

Requested changes made. Thanks for reviewing carefully enough to spot the error.

mozilla/gfx/thebes/src/gfxPangoFonts.cpp 1.78

I've fixed the pango bug pointed out in Comment 6 btw. In pango 1.26.1.

A standalone testcase that can be added to our test suite would be appreciated. https://developer.mozilla.org/en-US/docs/Mozilla/QA/Reducing_testcases