Closed Bug 623711 Opened 9 years ago Closed 9 years ago

erratic font selection with multiple @font-face rules and bad font resources

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: jfkthame, Assigned: jfkthame)

References

()

Details

Attachments

(2 files, 1 obsolete file)

(Reported via email by Adam Twardoch)

When visiting the above URL using Firefox on OS X, the renders Times at first, but after you zoom in with Cmd+ (and wait for the download to complete, if necessary), then Museo (the intended webfont) appears.

Pressing Cmd-R (reload) then makes the text revert to Times; zooming again restores the webfont.

This occurs because the site's CSS uses multiple @font-face rules with different src: formats; the last one specified (which is the first one Firefox tries) is SVG, which fails, so we fall back to the default font. But we also discard the font entry at this point, which means that when we reflow (on zoom, for example), we find a different font entry, and this time it loads successfully.

Bug 465414, bug 465450 and bug 494360 are all somewhat related to this behavior, though none of them specifically address the inconsistent rendering shown here.
This simplified testcase shows the same erratic behavior. When initially displayed, the middle line appears in Arial (which it should NOT); zooming the page causes it to switch to Courier (which is debatable, depending on the interaction of multiple @font-face rules). Reloading the page restores the (incorrect) Arial again.

Regardless of any decision about how repeated @font-face rules interact (override? accumulate?), it is surely wrong for the font selection to change as a result of reflow, in the way it does here.
Blocks: 494360
The basic problem here is that we discard gfxProxyFontEntry objects if all their sources fail to load; and then if a family ends up with no entries, we discard the entire family from the gfxUserFontSet. We shouldn't do that, because even if a @font-face source is unusable, the rule should still mask any platform family with the same name.
Assignee: nobody → jfkthame
Attachment #514754 - Flags: review?(jdaggett)
Comment on attachment 514754 [details] [diff] [review]
patch, don't discard proxy entries that fail to load

Argh, this patch breaks gfxPangoFonts; cancelling r? until I update it to fix that.
Attachment #514754 - Flags: review?(jdaggett)
Blocks: 633299
This version fixes the gfxPangoFont breakage. Currently, the Pango backend does not handle @font-face and fallback in the same way as other platforms, but I will file a separate bug on that.
Attachment #514754 - Attachment is obsolete: true
Attachment #515473 - Flags: review?(jdaggett)
Attachment #515473 - Flags: review?(jdaggett) → review+
Comment on attachment 515473 [details] [diff] [review]
patch, don't delete proxies that fail to load

Requesting approval-2.0? This fixes the erratic behavior described here that occurs because we allow user fonts to "disappear" from the font-set if all their sources fail to load. Required to conform to CSS3 Fonts, as well as to provide predictable behavior for users. The patch is pretty conservative: basically, it just ensures that the font entries stay around even after loading fails, so that font-matching and fallback behavior doesn't change "randomly" sometime after initial page-load.
Attachment #515473 - Flags: approval2.0?
Comment on attachment 515473 [details] [diff] [review]
patch, don't delete proxies that fail to load

Is this a regression? Minusing assuming that it isn't.
Attachment #515473 - Flags: approval2.0? → approval2.0-
Comment on attachment 515473 [details] [diff] [review]
patch, don't delete proxies that fail to load

(In reply to comment #6)
> Is this a regression? Minusing assuming that it isn't.

It's not a regression; FF3.6 shows the same behavior. But I do believe it's a pretty significant, user-visible bug that zooming the page (or other similar actions) can trigger a font change, as described in comments 0 and 1, and illustrated by the testcases here.

In view of the recent upsurge of interest in web fonts, I think it would be very unfortunate to ship FF4.0 with this bug unfixed, given that it's well-understood and we know how to fix it, and it can lead to bizarre, apparently-random rendering behavior for users.

Re-nominating for approval-2.0; please reconsider allowing us to take this.
Attachment #515473 - Flags: approval2.0- → approval2.0?
Comment on attachment 515473 [details] [diff] [review]
patch, don't delete proxies that fail to load

We can't fix all Web font bugs for FF4. Sorry :-(.
Attachment #515473 - Flags: approval2.0? → approval2.0-
This caused a reftest failure:

http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1300938685.1300939455.4272.gz

So, I backed it out:

http://hg.mozilla.org/projects/cedar/rev/72348a314f0b
Whiteboard: fixed-in-cedar → not-ready
Wrong patch backed out, so relanded:
https://hg.mozilla.org/projects/cedar/rev/533965e3b555

(For the real problem, see https://hg.mozilla.org/projects/cedar/rev/29f08082b76f .)
Whiteboard: not-ready → fixed-in-cedar
http://hg.mozilla.org/mozilla-central/rev/533965e3b555
http://hg.mozilla.org/mozilla-central/rev/29f08082b76f
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: fixed-in-cedar
Target Milestone: --- → mozilla2.2
Blocks: 653098
Depends on: 678774
No longer depends on: 678774
Depends on: 682824
No longer depends on: 682824
You need to log in before you can comment on or make changes to this bug.