Bug 65087 – Fix CSSFontSelector::addFontFaceRule()

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: New Bugs

[Patrick R. Gansterer]

Fix CSSFontSelector::addFontFaceRule()

[Patrick R. Gansterer]

Created attachment 101837 [details]
Patch

[Patrick R. Gansterer]

Can you have a look at the patch attached at this bug please.

I found a weird line of code added in http://trac.webkit.org/changeset/26484 while working on an other issue.
I don't know what this code (not) does exactly, but it doesn't look sane to me.

Good catch!

I know what the code is supposed to do: it’s supposed to make this work:

<style>
    @font-face {
        font-family: sans-serif;
        src: local(ahem);
    }
</style>
<div style="font-family: sans-serif;">This should be in Ahem</div>

However, looking at the working draft at <http://www.w3.org/TR/2011/WD-css3-fonts-20110324/#descdef-font-family> it seems as if generic family names are not valid values for the font-family font descriptor (it also doesn’t accept lists anymore…), so I’m not sure we should make the above snippet work. If we don’t, then of course we should just remove the whole thing.

The current code attempts (but as you’ve noticed, fails) to implement this earlier working draft: <http://www.w3.org/TR/2002/WD-css3-webfonts-20020802/#font-family>.

[Darin Adler]

Comment on attachment 101837 [details]
Patch

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

r=me on the idea of making the code work, but clearly we need regression tests before we actually make a change.

And we need to consider what behavior you want as you and Mitz discussed above.

> Source/WebCore/ChangeLog:10
> +        The varibable "familyName" is declared twice in this function.
> +        The second decleration hides the first one and makes the whole
> +        code in the the second useless.

You mean "defined", not "declared".

varibable -> variable
decleration -> declaration

[Patrick R. Gansterer]

Fix landed in http://trac.webkit.org/changeset/108366

*** This bug has been marked as a duplicate of bug 79103 ***