Bug 65087 - Fix CSSFontSelector::addFontFaceRule()
Summary: Fix CSSFontSelector::addFontFaceRule()
Status: RESOLVED DUPLICATE of bug 79103
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Patrick R. Gansterer
Depends on:
Reported: 2011-07-24 15:22 PDT by Patrick R. Gansterer
Modified: 2013-01-04 12:20 PST (History)
4 users (show)

See Also:

Patch (1.69 KB, patch)
2011-07-24 15:29 PDT, Patrick R. Gansterer
darin: review+
paroga: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick R. Gansterer 2011-07-24 15:22:40 PDT
Fix CSSFontSelector::addFontFaceRule()
Comment 1 Patrick R. Gansterer 2011-07-24 15:29:03 PDT
Created attachment 101837 [details]
Comment 2 Patrick R. Gansterer 2011-07-24 15:37:54 PDT
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.
Comment 3 mitz 2011-07-24 16:54:06 PDT
Good catch!

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

    @font-face {
        font-family: sans-serif;
        src: local(ahem);
<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.
Comment 4 mitz 2011-07-24 16:55:41 PDT
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>.
Comment 5 Darin Adler 2011-07-24 17:40:58 PDT
Comment on attachment 101837 [details]

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
Comment 6 Patrick R. Gansterer 2013-01-04 12:20:22 PST
Fix landed in http://trac.webkit.org/changeset/108366

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