Bug 194895 – Incorrect use of String::foldCase for font family names

• Status: RESOLVED
• Resolution: FIXED
• Component: Text

[Darin Adler]

Incorrect use of String::foldCase for font family names

[Darin Adler]

Created attachment 362589 [details]
Patch

[Darin Adler]

Myles, could you review this?

[Darin Adler]

Created attachment 362591 [details]
Patch

[Myles C. Maxfield]

Comment on attachment 362591 [details]
Patch

Looks good. https://drafts.csswg.org/css-fonts-4/#localized-name-matching says font names can be non-ascii but it looks like this code handles that case correctly.

[Darin Adler]

(In reply to Myles C. Maxfield from comment #4)
> Looks good. https://drafts.csswg.org/css-fonts-4/#localized-name-matching
> says font names can be non-ascii but it looks like this code handles that
> case correctly.

Actually, that draft says that we need to do full Unicode case folding. If we wanted to conform to that, then we’d have to change our hash and equality computation to use full Unicode case folding, too. We don’t have an efficient implementation of that but we could still do it.

My patch goes the other direction and does only ASCII case insensitive matching, which won’t fold non-ASCII characters. In most cases this is what we want, but the CSS specification specifically calls for the full case folding, sort of the opposite of what my patch does!

Now I don’t know what to do.

[Darin Adler]

Comment on attachment 362591 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=362591&action=review

> Source/WebCore/platform/graphics/FontCascadeDescription.cpp:127
>      return ASCIICaseInsensitiveHash::equal(family1, family2);

Writing a version of this for the Unicode case insensitivity algorithm would probably require using u_strCaseCompare. We don’t have any helpers for this because it’s not done anywhere else in WebKit, intentionally.

> Source/WebCore/platform/graphics/FontCascadeDescription.cpp:136
>      return ASCIICaseInsensitiveHash::hash(family);

Writing a version of this for the Unicode case insensitivity algorithm would be quite difficult. Might just need to call foldCase and then call hash, which would allocate/deallocate memory and be pretty slow!

[Darin Adler]

I wonder if we could get the standard changed. Requiring full case folding doesn’t really match the rest of the web platform. Things tend to be either case sensitive or ASCII case insensitive. Full Unicode case insensitivity is almost unheard of.

[Darin Adler]

Committed r242038: <https://trac.webkit.org/changeset/242038>

[Darin Adler]

Despite these issues I decided to land as-is. A correct implementation of ASCII case insensitivity seems better than an inconsistent implementation for now.

Outstanding issues remain: The specification says it should be full Unicode case folding. It's likely important that this logic exactly match what the underlying CoreText font name matching algorithm is on macOS and iOS. The CSS specification also talks about support for multiple names for the same font.

[Radar WebKit Bug Importer]

<rdar://problem/48361667>

[Myles C. Maxfield]

(In reply to Darin Adler from comment #9)
> Despite these issues I decided to land as-is. A correct implementation of
> ASCII case insensitivity seems better than an inconsistent implementation
> for now.
> 
> Outstanding issues remain: The specification says it should be full Unicode
> case folding. It's likely important that this logic exactly match what the
> underlying CoreText font name matching algorithm is on macOS and iOS. The
> CSS specification also talks about support for multiple names for the same
> font.

There’s a CSS F2F meeting this week; I’ll bring this up there. I agree with you about the desired behavior.

[Myles C. Maxfield]

https://github.com/w3c/csswg-drafts/issues/3675