Fix CSSFontSelector::addFontFaceRule()
Created attachment 101837 [details] Patch
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>.
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
Fix landed in http://trac.webkit.org/changeset/108366 *** This bug has been marked as a duplicate of bug 79103 ***