Summary: | [@font-face] font-family descriptor with multiple names should be discarded | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Philippe Wittenbergh <phiw2> | ||||||||
Component: | Layout and Rendering | Assignee: | 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
Philippe Wittenbergh
2009-06-03 22:02:37 PDT
Created attachment 67890 [details]
Patch
Comment on attachment 67890 [details]
Patch
Seems like this would break tests that use platform/win/css2.1/resources/Mac-compatible-font-fallback.css.
Created attachment 68190 [details]
Patch
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. Ping? Ping again? Did you find a way to run regression tests? 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 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 Created attachment 75655 [details]
Resolve merge conflict
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 Update the patch to resolve merge conflict. Can you review again? Comment on attachment 75655 [details]
Resolve merge conflict
OK.
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 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 on attachment 75655 [details]
Resolve merge conflict
Looks like a cq flake.
Comment on attachment 75655 [details] Resolve merge conflict Clearing flags on attachment: 75655 Committed r74401: <http://trac.webkit.org/changeset/74401> All reviewed patches have been landed. Closing bug. Have you tested against the relevant tests in the CSS 2.1 test suite? <http://test.csswg.org/suites/css2.1/20100917/> (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? |