Bug 26183

Summary: [@font-face] font-family descriptor with multiple names should be discarded
Product: WebKit Reporter: Philippe Wittenbergh <phiw2>
Component: Layout and RenderingAssignee: Yuzo Fujishima <yuzo>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, simon.fraser, webkit.review.bot, yuzo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
URL: http://devfiles.myopera.com/articles/695/webfonts.html
Attachments:
Description Flags
Patch
none
Patch
none
Resolve merge conflict none

Description Philippe Wittenbergh 2009-06-03 22:02:37 PDT
The linked page specifies an @font-face rule as:

@font-face {
    font-family :'myfont', serif;
    src: 'myfont.ttf';
}

p {font-family: 'myfont'; }

http://dev.w3.org/csswg/css3-fonts/#font-family0
(latest editor draft)

specifies font-family in a @font-face rule as
Value: <family-name> 

only one <family-name> is allowed.

On the testcase:
Actual result: the font specified in the @font-face rule is used
Expected: the default font is used.

Gecko (INVALID) bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=496128

see the last two comments there.

Opera 10b also suffers from the same issue.
Comment 1 Yuzo Fujishima 2010-09-17 00:57:02 PDT
Created attachment 67890 [details]
Patch
Comment 2 mitz 2010-09-17 01:29:45 PDT
Comment on attachment 67890 [details]
Patch

Seems like this would break tests that use platform/win/css2.1/resources/Mac-compatible-font-fallback.css.
Comment 3 Yuzo Fujishima 2010-09-20 23:11:37 PDT
Created attachment 68190 [details]
Patch
Comment 4 Yuzo Fujishima 2010-09-20 23:18:50 PDT
Thank you for the review.

(In reply to comment #2)
> (From update of attachment 67890 [details])
> Seems like this would break tests that use platform/win/css2.1/resources/Mac-compatible-font-fallback.css.

Rewrote the rules not to specify multiple families.

I didn't test it locally because running layout tests on Windows requires non-free font manipulation tool(s)
as far as I can tell from http://trac.webkit.org/wiki/BuildingOnWindows

I did search the files under LayoutTests and confirmed that I don't need to change other files.
Comment 5 Yuzo Fujishima 2010-10-29 00:12:55 PDT
Ping?
Comment 6 Yuzo Fujishima 2010-11-21 22:21:06 PST
Ping again?
Comment 7 Alexey Proskuryakov 2010-11-22 12:58:14 PST
Did you find a way to run regression tests?
Comment 8 Eric Seidel (no email) 2010-12-03 10:53:37 PST
Comment on attachment 68190 [details]
Patch

Seems reasonable to me.  The cq will run layout tests if that was the above concern?  I'll set r+ and someone else can set cq+ to have the tests run and landed.
Comment 9 WebKit Commit Bot 2010-12-05 18:19:00 PST
Comment on attachment 68190 [details]
Patch

Rejecting patch 68190 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=abarth-cq-sl', 'apply-attachment', '--force-clean', '--non-interactive', 68190]" exit_code: 2
Last 500 characters of output:
escriptor.html
patching file LayoutTests/platform/win/css2.1/resources/Mac-compatible-font-fallback.css
patching file WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebCore/css/CSSParser.cpp
Hunk #1 FAILED at 5493.
Hunk #2 succeeded at 5573 (offset 73 lines).
1 out of 2 hunks FAILED -- saving rejects to file WebCore/css/CSSParser.cpp.rej

Failed to run "[u'/Users/abarth/git/webkit-queue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Eric Seidel', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/6776048
Comment 10 Yuzo Fujishima 2010-12-06 00:37:09 PST
Created attachment 75655 [details]
Resolve merge conflict
Comment 11 WebKit Commit Bot 2010-12-06 01:35:14 PST
Comment on attachment 75655 [details]
Resolve merge conflict

Rejecting patch 75655 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'land-attachment', '--force-clean', '--ignore-builders', '--non-interactive', '--parent-command=commit-queue', 75655]" exit_code: 1
Last 500 characters of output:
75655&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=26183&ctype=xml
Processing 1 patch from 1 bug.
Cleaning working directory
Updating working directory
Processing patch 75655 from bug 26183.
NOBODY (OOPS!) found in /Projects/CommitQueue/LayoutTests/ChangeLog does not appear to be a valid reviewer according to committers.py.
ERROR: /Projects/CommitQueue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/6748071
Comment 12 Yuzo Fujishima 2010-12-06 02:30:45 PST
Update the patch to resolve merge conflict. Can you review again?
Comment 13 Eric Seidel (no email) 2010-12-09 23:36:53 PST
Comment on attachment 75655 [details]
Resolve merge conflict

OK.
Comment 14 WebKit Commit Bot 2010-12-10 01:58:24 PST
Comment on attachment 75655 [details]
Resolve merge conflict

Rejecting patch 75655 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-sf', 'build', '--no-clean', '--no-update', '--build-style=both']" exit_code: 2
Last 500 characters of output:
env YACC /Developer/usr/bin/yacc
    /bin/sh -c /Projects/CommitQueue/WebKitBuild/WebCore.build/Debug/WebCore.build/Script-5DF50887116F3077005202AB.sh

** BUILD FAILED **


The following build commands failed:
WebCore:
	CompileC /Projects/CommitQueue/WebKitBuild/WebCore.build/Debug/WebCore.build/Objects-normal/x86_64/DOMSVGAnimatedInteger.o /Projects/CommitQueue/WebKitBuild/Debug/DerivedSources/WebCore/DOMSVGAnimatedInteger.mm normal x86_64 objective-c++ com.apple.compilers.gcc.4_2
(1 failure)


Full output: http://queues.webkit.org/results/7025005
Comment 15 Eric Seidel (no email) 2010-12-14 01:24:42 PST
Comment on attachment 68190 [details]
Patch

Cleared Eric Seidel's review+ from obsolete attachment 68190 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 16 Eric Seidel (no email) 2010-12-20 22:47:45 PST
Comment on attachment 75655 [details]
Resolve merge conflict

Looks like a cq flake.
Comment 17 WebKit Commit Bot 2010-12-20 23:30:24 PST
Comment on attachment 75655 [details]
Resolve merge conflict

Clearing flags on attachment: 75655

Committed r74401: <http://trac.webkit.org/changeset/74401>
Comment 18 WebKit Commit Bot 2010-12-20 23:30:32 PST
All reviewed patches have been landed.  Closing bug.
Comment 19 Simon Fraser (smfr) 2010-12-20 23:33:40 PST
Have you tested against the relevant tests in the CSS 2.1 test suite? <http://test.csswg.org/suites/css2.1/20100917/>
Comment 20 Yuzo Fujishima 2011-01-05 01:35:48 PST
(In reply to comment #19)
> Have you tested against the relevant tests in the CSS 2.1 test suite? <http://test.csswg.org/suites/css2.1/20100917/>

No.

I don't see any tests that are very relevant to this patch there.
Comma-separated font-family "properties" (not "descriptor") seem to be working,
as in http://test.csswg.org/suites/css2.1/20100917/html4/c522-font-family-000.htm .  

Any specific tests you are interested in testing?