Something in CSS serializer must have changed recently in the last 6 months or so. We now produce single quotation around font family names: e.g. <font face="'trebuchet ms', sans-serif">something</font> instead of <font face="trebuchet ms, sans-serif">something</font> and Outlook 2007 fails to render the latter properly. A very simple fix is to strip any single quotation that appears in the font family name when creating a font element. Even though font family name could contain a quotation mark in the theory, that shouldn't be a problem in practice.
The spec requires quotes for font names if they don't consist of a sequence of one or more identifiers separated by single spaces, or if a font happens to be named the same thing as one of the reserved keywords: http://www.w3.org/TR/CSS21/fonts.html#font-family-prop But "font-family: trebuchet ms, sans-serif" is valid without quotes around "trebuchet ms".
IIRC, browsers are very inconsistent here. If we're going to add quotations back in, we should double check what IE/Opera/Firefox do. Specifically, do they use double or single quotes?
Caused by changes in bug 64783?
<rdar://problem/10929517>
(In reply to comment #3) > Caused by changes in bug 64783? The bug 64783 is fixing the CSS parser though, not the CSS serializer.
Created attachment 128888 [details] Screenshot: old create-models page
Created attachment 128889 [details] Screenshot: Old merge-tests page
Created attachment 128890 [details] Screenshot: shiny(tm) new admin page
Oops, sorry, these screenshots are for the bug 79585 :(
Created attachment 128892 [details] Needs a rubber stamp
Comment on attachment 128892 [details] Needs a rubber stamp Ugh... wrong bug again :(
I'll take a look.
valueForFamily() in CSSComputedStyleDeclaration.cpp creates CSSPrimitiveValue based on the given faily name with type CSS_STRING. CSSPrimitiveValue::customCssText() returns single-quoted value if the type is CSS_STRING and the value contains spaces. I couldn't find when the behavior changed. In r80463, which was landed March 2011, valueForFamily() creates CSSPrimitiveValue with type CSS_STRING and CSSPrimitiveValue::customCssText() quotes the value. Maybe we can fix the regression by introducing a new primitive type and . Not sure it's worth to do.
(In reply to comment #13) > Maybe we can fix the regression by introducing a new primitive type and . ... and just return the value as is in customCSssText().
Created attachment 129629 [details] Patch
(In reply to comment #15) > Created an attachment (id=129629) [details] > Patch I'm not sure this is really a regression, though.
Comment on attachment 129629 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129629&action=review > Source/WebCore/css/CSSPrimitiveValue.cpp:844 > + text = m_value.string; ?? We can't do this per Aryeh's comment, right? If you have something like A ! B as a font name, then we need to quote it.
Created attachment 129645 [details] Patch
Comment on attachment 129645 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129645&action=review > LayoutTests/editing/style/font-face-unquote.html:9 > +<script src="script-tests/font-face-unquote.js"></script> Please use http://trac.webkit.org/browser/trunk/Tools/Scripts/make-new-script-test instead so that you don't have a separate js file.
Created attachment 129648 [details] Patch
Comment on attachment 129645 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129645&action=review Thank you for review. >> LayoutTests/editing/style/font-face-unquote.html:9 >> +<script src="script-tests/font-face-unquote.js"></script> > > Please use http://trac.webkit.org/browser/trunk/Tools/Scripts/make-new-script-test instead so that you don't have a separate js file. Done.
Comment on attachment 129648 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129648&action=review > Source/WebCore/ChangeLog:8 > + Reviewed by NOBODY (OOPS!). This line should appear before the description. > LayoutTests/ChangeLog:9 > + Reviewed by NOBODY (OOPS!). Ditto. > LayoutTests/editing/style/font-face-unquote-expected.txt:1 > +Test to make sure WebKit doesn't quote font face. in face content attribute? I mean we do quote them in font-family property value, right? > LayoutTests/editing/style/font-face-unquote.html:6 > +<head> > +<meta charset="utf-8"> > +<script src="../../fast/js/resources/js-test-pre.js"></script> > +</head> You can just put the script element in body and there's no need for the meta.
Thanks for the fix! Please also comment on the google bug I cc'ed you. (This was a Gmail bug).
Created attachment 129656 [details] Patch for landing
Comment on attachment 129648 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129648&action=review Thanks! >> Source/WebCore/ChangeLog:8 >> + Reviewed by NOBODY (OOPS!). > > This line should appear before the description. Done. >> LayoutTests/ChangeLog:9 >> + Reviewed by NOBODY (OOPS!). > > Ditto. Done. >> LayoutTests/editing/style/font-face-unquote-expected.txt:1 >> +Test to make sure WebKit doesn't quote font face. > > in face content attribute? I mean we do quote them in font-family property value, right? You are right. Changed the description. >> LayoutTests/editing/style/font-face-unquote.html:6 >> +</head> > > You can just put the script element in body and there's no need for the meta. Done.
Comment on attachment 129656 [details] Patch for landing Rejecting attachment 129656 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/11767379
Created attachment 129678 [details] Patch for landing
Comment on attachment 129678 [details] Patch for landing Rejecting attachment 129678 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/11766434
Comment on attachment 129678 [details] Patch for landing Try again.
Comment on attachment 129678 [details] Patch for landing Clearing flags on attachment: 129678 Committed r109382: <http://trac.webkit.org/changeset/109382>
All reviewed patches have been landed. Closing bug.
*** Bug 71812 has been marked as a duplicate of this bug. ***