476378 - Soft hyphens rendered incorrectly when chosen font lacks U+2010 HYPHEN and fallback font is sufficiently different

Status: VERIFIED FIXED

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

Description

[Shreevatsa R]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-GB; rv:1.9.0.5) Gecko/2008120121 Firefox/3.0.5
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-GB; rv:1.9.0.5) Gecko/2008120121 Firefox/3.0.5

Soft hyphens ("shy": ­) are rendered as a character that looks like "रू" (रू). If I copy the rendered character, it is correctly copied as the soft-hyphen character.

This is not because of a broken font: it happens with most fonts, including Andale Mono, Arial, Baskerville [Old Face], Comic Sans MS, Courier New, Futura, Garamond, Geneva CY, Georgia, Gill Sans, Helvetica CY, Helvetica Neue, Hoefler Text, Lucida Bright, Lucida Sans, Microsoft Sans Serif, Optima, Palatino, Papyrus, Times, Times New Roman, Verdana, etc.

(but renders fine -- like a hyphen -- with American Typewriter, Arial Unicode MS, Courier, Geneva, Gentium, Helvetica, Lucida Grande, Monaco, Trebuchet MS and a few others.)

Reproducible: Always

Actual Results:  
Character at end of line looks like रू

Expected Results:  
Character should look like a hyphen

Comment 1

[Shreevatsa R]

Attachment: Example HTML file that is rendered incorrectly

Contains a soft hyphen after every character

Comment 2

[Shreevatsa R]

Attachment: Screenshot (with Times font)

This is how it is rendered, with the default font set to Times

Comment 3

[Shreevatsa R]

Attachment: Screenshot: how attachment 199178 is rendered

Just in case this is related: this is how Attachment 199178 [details] (of Bug 312063) is rendered on my system.

Comment 4

[Jesse Ruderman]

Works fine for me.  It appears as a hyphen.  I have my default font set to Times 16, which I think is the default default.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.5) Gecko/2008120121 Firefox/3.0.5

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090201 Minefield/3.2a1pre

"रू" is Devanagari, right?  Do you have any settings related to Devanagari or languages that use Devanagari (e.g. Hindi)?

Comment 5

[philippe (part-time)]

Renders fine here as well. This sounds a like bug 455243.

Comment 6

[Shreevatsa R]

Thanks for mentioning bug 455243 -- there was a particular font on my system removing which fixed the problem. However, unlike bug 455243:

 * That font was not the last on the list of fonts

 * The name of that font ("xdvng") was not similar in any way to Times (and it could not have been similar to all of those wide set of fonts anyway)

It's a pretty crazy font, but having it around ought not to interfere with existing good fonts (and it doesn't explain why rendering was good with one set of fonts and not with another).

Is there any further information that might be helpful?

Comment 7

[philippe (part-time)]

Could you test with a nightly build ? (with that 'crazy font' installed, ideally).

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/

Comment 8

[Shreevatsa R]

Yes, the problem is still present in the nightly build.

I called the font 'crazy' because it's some ad-hoc encoding that maps characters to Devanagari letters (see http://www-personal.umich.edu/~pehook/devnag.html ).
I've put the font at http://web.mit.edu/vatsa/Public/XDVNGm.ttf

Comment 9

[philippe (part-time)]

(In reply to comment #8)
> Yes, the problem is still present in the nightly build.
> 
> I called the font 'crazy' because it's some ad-hoc encoding that maps
> characters to Devanagari letters (see
> http://www-personal.umich.edu/~pehook/devnag.html ).
> I've put the font at http://web.mit.edu/vatsa/Public/XDVNGm.ttf
I downloaded that font and installed in ~/library/fonts (and restarted the browsers).
* Fx 3.05(Gecko 1.9.0.5) and Camino nightly (Gecko 1.9.0.7pre): I can see the problem
* Minefield latest nightly: no problems (tested with a default profile).

Comment 10

[Shreevatsa R]

I do see it with the Minefield latest nightly [ http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/firefox-3.2a1pre.en-US.mac.dmg ] and a newly created profile, so I don't know what to add :)
Maybe it depends on something else as well: what might it be?

Comment 11

[philippe (part-time)]

John, any idea ?

I my have some other (S-Asian) fonts that 'correct' the issue on trunk, but I doubt it.

Comment 12

[Jonathan Kew [:jfkthame]]

The problem arises when the current font doesn't support U+2010 HYPHEN, which we are using to render the soft hyphen when a break is actually taken at this position. If the font doesn't support that character, we look for a fallback font, and if the one we happen to find has some other random glyph (such as Devanagari RUU) at that position, it will of course look broken.

Unfortunately, there are many widespread fonts, including many of the system ones, that don't support the U+2010 codepoint, so fallback for soft-hyphen will be quite common. Fortunately, fonts that do "support" U+2010 but have an incorrect glyph at that codepoint are relatively rare.

Note that even if we get a fallback font that does provide a proper hyphen glyph, this situation is still not ideal because the design of the fallback hyphen will not properly match the current typeface. How glaring this is will depend on the particular fonts involved, of course.

While there's no way we can entirely avoid the risk of displaying garbage if the user has hacked, non-Unicode-compliant fonts installed, we could resolve this instance by rendering soft hyphens that are actually chosen as line breaks with U+002D HYPHEN-MINUS rather than U+2010 HYPHEN. This will virtually eliminate the use of fallback font matching for these, as U+002D is supported in virtually every font.

Comment 13

[Jonathan Kew [:jfkthame]]

Attachment: test file to show soft-hyphen rendering problem with Hoefler

This is a version of the test file that illustrates the font mismatch when using Hoefler, which has a distinctive high, thin hyphen character. The first paragraph has soft hyphens, and will use a fallback glyph at the line-break; the second paragraph shows real Hoefler hyphens for comparison.

Comment 14

[Jonathan Kew [:jfkthame]]

Attachment: screenshot of the Hoefler soft-hyphen test file

note the incorrect glyph for soft-hyphen on the first line

Comment 15

[Jonathan Kew [:jfkthame]]

Attachment: patch to correct soft-hyphen display error

This patch attempts to resolve the issue (not just the glaring problem caused by a bizarre hacked font, but also the more general font mismatch as shown in the Hoefler example) by only using U+2010 if it is supported by the first font in the current fontGroup; otherwise, it uses the ASCII hyphen-minus character instead.

Comment 16

[Jonathan Kew [:jfkthame]]

Attachment: the Hoefler soft-hyphen sample file, as displayed by patched Minefield

Comment 17

[Jonathan Kew [:jfkthame]]

Attachment: screenshot of test on Vista showing similar soft-hyphen font mismatch

This issue can show up on any platform; I've reproduced it on Vista using text with the style "font-family: Segoe UI, Trebuchet MS". The screenshot shows how soft-hyphen (in the first text run) falls back to the glyph from Trebuchet, which is much shorter and thicker than the hyphen (ASCII 0x2D) in Segoe UI.

Comment 18

[Jonathan Kew [:jfkthame]]

Note that this patch will cause the existing soft-hyphen reftests to start failing. I believe this is correct, because those tests have been using U+2010 in the reference text, and encountering font fallback in most cases (exact behavior could depend on the fonts on the test system).

We should fix those tests to use U+002D instead (I don't recall seeing any real-life fonts where U+2010 is actually meant to render differently from U+002D).

(An alternative would be to explicitly choose fonts that are known to support U+2010, but it may be difficult to do this reliably across all platforms and versions.)

Marking the patch for review; if there is agreement on the approach, then I'll incorporate fixes for the reftests, as well as a new reftest that is explicitly designed to check for this bug.

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #18)
> We should fix those tests to use U+002D instead (I don't recall seeing any
> real-life fonts where U+2010 is actually meant to render differently from
> U+002D).

Sigh. I felt full of Unicode righteousness for using U+2010. If this is the case, why don't we just use U+002D and simplify our code? I suppose because someone will promptly create a font that has different hyphens and file a bug against us.

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 360290 [details] [diff] [review]
patch to correct soft-hyphen display error

+  nsRefPtr<gfxFont> font = fontGroup->GetFontAt(0);

"gfxFont* font" is adequate here. Or you could just write fontGroup->GetFontAt(0)->HasCharacter ... we rely on the first font being there in many places.

Comment 21

[Jonathan Kew [:jfkthame]]

(In reply to comment #19)

> Sigh. I felt full of Unicode righteousness for using U+2010. If this is the
> case, why don't we just use U+002D and simplify our code? I suppose because
> someone will promptly create a font that has different hyphens and file a bug
> against us.

Yes, I considered doing that, but I figured that if a font does support U+2010 then using it is probably the right thing to do, and we wouldn't want to throw away your hard-earned righteousness! :)

I'll update the patch and check on the testcases, etc. I have a reftest for this bug, but as mentioned above, I know the patch will break others that we already have.

Comment 22

[Jonathan Kew [:jfkthame]]

Attachment: updated patch following review comments, added/fixed reftests

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 360460 [details] [diff] [review]
updated patch following review comments, added/fixed reftests

It would be nice if we had a test that checks U+2010 is used if it's available.

Comment 24

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed http://hg.mozilla.org/mozilla-central/rev/48bf047a9054

Should we take this on 1.9.1 I wonder?

Comment 25

[Jonathan Kew [:jfkthame]]

(In reply to comment #24)

> Should we take this on 1.9.1 I wonder?

I don't think it deserves to be a blocker, but OTOH the change looks extremely low-risk. The fix would definitely be nice to have, as our current behavior gives incorrect (and unpredictable) rendering; usually it's not too glaring but there's the possibility (as in the original bug report) of it being completely bizarre. So I'd be in favor of asking for 1.9.1 approval. What's the process for that at this stage?

Comment 26

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We should land this on 1.9.1 when the patch has baked a bit

Comment 27

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/3289ec4eeae7

Comment 28

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090721 Minefield/3.6a1pre ID:20090721044139

and

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1pre) Gecko/20090720 Shiretoko/3.5.1pre ID:20090720042942