Closed Bug 467228 Opened 13 years ago Closed 13 years ago

Letters at the ends of words appear stretched out in italic 'Hoefler Text' font

Categories

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

x86
macOS
defect

Tracking

()

VERIFIED FIXED
mozilla1.9.1b3

People

(Reporter: jruderman, Assigned: jfkthame)

References

()

Details

(Keywords: testcase, verified1.9.1)

Attachments

(4 files, 1 obsolete file)

Attached file testcase
In Firefox 3 and Firefox trunk on Mac, the ends of some words on http://www.slugsite.com/archives/957 appear stretched out.  This doesn't happen in Safari or Opera.
If you paste the sentence in the screenshot from Safari into a TextEdit .rtf document, you get the same result as what Gecko shows. Simply typing the same thing in that doc also gives that stretched alternate glyph for 'n'.

(From Safari, because that preserves all styles and doesn't mangle non ascii characters)
I strongly suspect that this is simply us supporting shaping for Latin text, while Safari and Opera do not, and that this is simply what the font wants. But Jonathan should check.

(In reply to comment #2)
> (From Safari, because that preserves all styles and doesn't mangle non ascii
> characters)

Is there a bug filed on that? We should be preserving styles now that bug 428096 has landed.
Yes, this is because we support "smart-font" shaping, and the others don't, and these are contextual alternates from Hoefler Text Italic. However, the behavior shown isn't really what the font wants, it is an artifact of how we're interacting with the platform layout engine.

These swash glyphs in Hoefler should be part of the Line Final Swashes setting of the Smart Swashes feature, and are only supposed to appear at end of line. (Aside: for some reason, I'm not seeing this in the Typography palette of TextEdit at the moment. But that's probably Apple's problem.) However, whenever we pass a run of text to ATSUI (or CoreText) to create gfxGlyphRuns, the end of that text fragment will be considered an end-of-line; at that point, we don't know where on the line it will actually occur.

Until we are prepared to do some pretty major work to cope with the fact that shaping and metrics may be affected by line boundaries - which has implications for the textrun cache and for the whole measurement and line-breaking algorithm - I think we should simply turn off the Line Initial and Line Final swash features through the ATSUI/CoreText APIs. It's better to disable these altogether than to have them popping up at apparently-random word endings throughout the text, depending how the strings happen to get divided up and passed to the platform.

I'll experiment with a patch to do this.
I've created a patch to disable these swashes when using ATSUI. With this, the test case (and original site) render OK. I'll see if I can come up with a reftest for this (it's a bit tricky to figure out how to do a "reference" version that will render consistently with or without the fix).
Attachment #350680 - Flags: review?(roc)
Attached patch possible reftest (obsolete) — Splinter Review
This tests that line-final swashes are disabled by comparing "Free" with "Free$", where the "$" is transparent.  It fails with a current build, but I haven't tested that it passes with the patch.
That's great, thanks. I've checked that it fails without the patch and passes with it.

I first tried a somewhat different approach to building a test, but it ran afoul of the fact that the reference and test windows share the gfxTextRun cache. That's worth keeping in mind; I had a test file and a -ref that each behaved as I expected in isolation, but when run together under reftest, they influenced each other such that the test would always pass!
 
> (In reply to comment #2)
> > (From Safari, because that preserves all styles and doesn't mangle non ascii
> > characters)
> 
> Is there a bug filed on that? We should be preserving styles now that bug
> 428096 has landed.
Bug 466599
Instead of using that test as-is, I think it would be better to put something after the text on the line so that if/when we add line-end shaping we don't have to fix the test. Some font-size:0 text should be OK.
OK, with the suggested addition, the test should be stable even if we add line-end contextual behavior someday. Also exchanged the test and reference files as I realized they were reversed; the "bad" behavior was occurring in the -ref version!
Attachment #350708 - Flags: review?(roc)
Comment on attachment 350680 [details] [diff] [review]
disable line-initial/final swashes when creating ATSUI styles
[Checkin: Comment 14 & 17]

We should take this for 1.9.1 because currently it's semi-random, caching-dependent behaviour --- that's always bad.
Attachment #350680 - Flags: approval1.9.1?
This can land in 1.9.2.
Keywords: checkin-needed
Whiteboard: [needs landing]
Assignee: nobody → jfkthame
Pushed 7baaa800925d to trunk
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 191 approval]
Tests committed to trunk as 9e1eab6135e2
Flags: in-testsuite+
Attachment #350708 - Attachment description: updated version of the swash reftest, as suggested in comment 10 → updated version of the swash reftest, as suggested in comment 10 [Checkin: Comment 15]
Attachment #350680 - Attachment description: disable line-initial/final swashes when creating ATSUI styles → disable line-initial/final swashes when creating ATSUI styles [Checkin: Comment 14]
Attachment #350682 - Attachment is obsolete: true
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9.2a1
Comment on attachment 350680 [details] [diff] [review]
disable line-initial/final swashes when creating ATSUI styles
[Checkin: Comment 14 & 17]

a191=beltzner, please make sure that you also check in the tests
Attachment #350680 - Flags: approval1.9.1? → approval1.9.1+
Whiteboard: [needs 191 approval] → [needs 191 landing]
Flags: wanted1.9.1? → blocking1.9.1+
Priority: -- → P3
Comment on attachment 350680 [details] [diff] [review]
disable line-initial/final swashes when creating ATSUI styles
[Checkin: Comment 14 & 17]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ac39002d305c
Attachment #350680 - Attachment description: disable line-initial/final swashes when creating ATSUI styles [Checkin: Comment 14] → disable line-initial/final swashes when creating ATSUI styles [Checkin: Comment 14 & 17]
Comment on attachment 350708 [details] [diff] [review]
updated version of the swash reftest, as suggested in comment 10
[Checkin: Comment 15 & 18]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/30dcf7ca8d06
Attachment #350708 - Attachment description: updated version of the swash reftest, as suggested in comment 10 [Checkin: Comment 15] → updated version of the swash reftest, as suggested in comment 10 [Checkin: Comment 15 & 18]
Whiteboard: [needs 191 landing]
Target Milestone: mozilla1.9.2a1 → mozilla1.9.1b3
verified Fixed using the attached testcase on builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090721 Minefield/3.6a1pre ID:20090721031556

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.2pre) Gecko/20090721 Shiretoko/3.5.2pre ID:20090721075223
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.