35824 - DeviceContextImpl::FirstExistingFont called wrongly: callers discard font alias information

Status: RESOLVED WONTFIX

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Chris Siebenmann]

DeviceContextImpl::FirstExistingFont (in gfx/src/nsDeviceContext.cpp)
is (or appears to be) called to find the first of a list of font families
actually exist on the system, and also check for font aliases that map
various commonly used font names around that are set up in
DeviceContextImpl::CreateFontAliasTable.

It's prototyped as:
	FirstExistingFont(const nsFont& aFont, nsString& aFaceName);
and in practice returns the font family it has found for you and wants
you to use in aFaceName.

 Unfortunately, the layout code doesn't use it this way. It calls it
and then doesn't look at the aFaceName. This has the effect of discarding
any font aliases or other clever things that FirstExistingFont() has
found for it, completely defeating the careful font aliasing that's
set up in nsDeviceContext.cpp (and I think elsewhere on some platforms).
Oops.

 The most visible symptom of the bug is that a web page that uses
a 'FONT FACE=...' with a name that is available as a customary alias
(eg 'Arial' when the system has no Arial but does have a Helvetica)
will not use this available font, but instead fall back onto the user's
default browser font.

 Places that do this:
- layout/html/content/src/nsHTMLFontElement.cpp in MapFontAttributesInto
- layout/html/style/src/nsCSSStyleRule.cpp in MapDeclarationFontInto

 Source: recent (today) CVS pull. I can't see anything in today's checkins
that would change this.

 Hopefully I have got all this right. I have made a Linux build that shows
this bug in action  (one has to make a small change in the GTK gfx layer to
show it), so I'm pretty sure the bug is real.

Comment 1

[troy]

Changing component to Style System since it looks like that's where it is called 
from

Comment 2

[Pierre Saslawsky]

The aliases are not needed at that point. The familyList is preserved and that's 
what matters. Still, you raised a big doubt in my mind and I did the following 
check on the Mac (you can easily adapt for other platforms).

In nsDeviceContextMac::CreateFontAliasTable, add:
     nsAutoString  foo;  foo.AssignWithConversion("Foo");
     nsAutoString  chicago;  chicago.AssignWithConversion("Chicago");
  and:
     AliasFont(foo, chicago, fontNullStr, PR_FALSE);

Then load the following HTML:
    <font face="Foo" size=7>
        This Foo should appear as Chicago
    </font>

Result: the text is displayed in Chicago, not in the default serif font. It 
works. Closed as Invalid.

Comment 3

[Chris Siebenmann]

 I hate to be a fly in the ointment, but: this definetly doesn't happen
under GTK. GTK has no specific re-implementation of this class function,
but the generic DeviceContextImpl one gets run, and it doesn't work (and
when I put in my hack workaround in the two layout functions, it does
work); I tested with a 'Foo' alias for a very visually distinctive font
I have, so I'm sure of this.

 Quite possibly this means that GTK itself has a bug in how it handles
this: is the low-level (GTK, Mac, etc) code supposed to look up the alias
again itself when it's getting handed a font to look up, or otherwise
handle it? I believe that the GTK code just looks up whatever name it
gets handled and punts if it's not there.

Comment 4

[Pierre Saslawsky]

Thanks a bunch for verifying on your favorite platform, I should have done it 
myself but I wrongly assumed that the bilingual Windows/Unix folks had kept the 
GTK up-to-date.

CCd Erik, just for info. Reassigned to the GTK guy, Pavlov, to implement or fix 
the FontAliasTable on Unix.

Comment 5

[Erik van der Poel]

The font-related methods in nsIDeviceContext need to be cleaned up, possibly by
removing them. The platform-specific code under gfx/src/{windows,gtk,etc}
performs all of the "first existing font"-type stuff. Aliases are also mapped
down there. I don't know about the Mac, but Windows and Unix (GTK) have the
aliases down there.

There is a bug in the gfx/gtk code, though. It currently maps names like Times
New Roman to "times" without even checking whether Times New Roman is available
on the system (and it might be, since some people have TrueType facilities).
This is bug 31013, which I will fix shortly. I'll look into this bug at the
same time. Re-assigning to myself.

Comment 6

[Erik van der Poel]

The callers (nsHTMLFontElement and nsCSSStyleRule) only appear to be interested
in whether or not the first existing font is "-moz-fixed". These callers ignore
any other info, such as aliasing.

However, I believe the platforms are performing their own aliasing when font
metrics objects are initialized or used for measuring or drawing.

There is a problem with the FirstExistingFont code in nsDeviceContext.cpp. It
first tries the aliases before checking to see whether that font is available.
This is a problem because e.g. Linux can have "Times New Roman" installed if the
user has TrueType facilities, so we don't want to map that name to "times". We
want to try "Times New Roman" first.

The Unix-specific (GTK-specific) code does try the names first, before looking
up aliases (now that I've checked in that fix). So, it would seem like some of
this XP code should just go away, but FirstExistingFont is still being called
(to check for "-moz-fixed") so I guess I'll leave it there for now.

I'm moving this bug to a late milestone, as a reminder to perform further clean
up (if necessary) when we have the time.

Comment 7

[Chris Siebenmann]

 Unless I'm hallucinating, FirstExistingFont is careful to check if the
alias *doesn't* exist before it sets it up (and, I think, that the 'real'
font also exists), unless you tell it to force the alias. (Used once, which
I maintain is a mistake/bug in the code.) It's slick code; it's just not
getting used by (at least) GTK.

Comment 8

[Erik van der Poel]

If the document or style sheet requests Arial, doesn't UnixMoz give "helvetica"
on systems that don't have Arial? If this currently works, I'd rather leave the
FirstExistingFont code alone for now. (I'm trying to stay focussed on pressing
problems.)

Comment 9

[Hixie (not reading bugmail)]

Netscape's standard compliance QA team reorganised itself once again, so taking 
remaining non-tables style bugs. Sorry about the spam. I tried to get this done 
directly at the database level, but apparently that is "not easy because of the 
shadow db", "plus it screws up the audit trail", so no can do...

Comment 10

[Frank Tang]

erik resign. reassign all his bug to ftang for now.

Comment 11

[Frank Tang]

mark all future new as assigned after move from erik to ftang

Comment 12

[Olav Vitters]

*** Bug 143641 has been marked as a duplicate of this bug. ***

Comment 13

[Frank Tang]

what a hack. I have not touch mozilla code for 2 years. I didn't read these bugs
for 2 years. And they are still there. Just close them as won't fix to clean up.

Comment 14

[Travis Chase]

Mass Bug Re-Open of bugs Frank Tang Closed with no good reason. Spam is his
fault not my own

Comment 15

[Travis Chase]

Mass Re-assinging Frank Tangs old bugs that he closed won't fix and had to be
re-open. Spam is his fault not my own

Comment 16

[Zack Weinberg (:zwol)]

The font code is totally different now; this bug is no longer useful.