Some families, for example M+ (http://mplus-fonts.sourceforge.jp/mplus-outline-fonts/design/index-en.html) do not provide an explicit italic/oblique font. Firefox behaves like the system does, by emulating italic using slanted Roman (more likely Pango does that automatically), in WebKit, regular roman is used instead. (Sorry if this is reported against an incorrect element.)
This happens both in GTK port and current Chromium snapshots. No idea about other ports.
Chromium bug: http://code.google.com/p/chromium/issues/detail?id=31833
Here is a small html that could reproduce the problem. <html><head> <style type="text/css"> @font-face { font-family: "A"; src: url("http://HOST.NAME/PATH/TO/FONT.ttf"); } .test { font-family: 'A'; } </style></head> <body><div> <strong><p class="test">abcde</p></strong> <i><p class="test">abcde</p></i> <strong><i><p class="test">abcde</p></i></strong> <p class="test">abcde</p> <p>abcde</p> </div></body></html> When we don't specify font-weight: in a @font-face rule, WebCore/css/CSSFontSelector.cpp treat the font as if it has 100|200|300|400|...|900 weight. Not 400 (i.e., normal weight). And when a @font-face font has the 100|...|900 weight mask, current WebCore/css/CSSSegmentedFontFace.cpp implementation never synthesize a bold font for the font. That seems to be the problem. The same is true for italic. Unusual mask is used when font-style: is missing from @font-face. I think we could fix the problem by giving special treatment to the 100|...|900 mask in CSSSegmentedFontFace.cpp. (And I think we shouldn't modify CSSFontSelector.cpp side, since the 100|...|900 mask is necessary for proper handling of this case: https://bugs.webkit.org/show_bug.cgi?id=16348 Without the 100|...|900 mask, <i>...</i> text in https://bugs.webkit.org/attachment.cgi?id=28455&action=prettypatch will be displayed in Times, not Arial.) I'll attach a patch with tests shortly.
Created attachment 48475 [details] bold_italic_font_face_v1
Comment on attachment 48475 [details] bold_italic_font_face_v1 > +++ b/LayoutTests/platform/mac/fast/css/font-face-synthetic-bold-italic-expected.txt > @@ -0,0 +1,48 @@ What part of this result represents the correctness of this patch?
Comment on attachment 48475 [details] bold_italic_font_face_v1 WebKit’s current behavior matches the working draft that allowed specifying multiple values for the font-weight and font-style descriptors: <http://www.w3.org/TR/2002/WD-css3-webfonts-20020802/#select>. The latest working draft does not allow it <http://www.w3.org/TR/2009/WD-css3-fonts-20090618/#font-property-descriptors-the-font-style>. I think it’s reasonable to change the behavior to match the newer draft—which I think will have the same effect on the test case in this patch. However, this patch introduces new behavior that doesn’t match either version of the spec. That seems like a bad idea.
Created attachment 48787 [details] bold_italic_font_face_v2
(In reply to comment #6) Thanks for the comment. I've updated the patch so that it follows the latest 2009 draft. Can you please take another look? > What part of this result represents the correctness of this patch? Kent-san, I modified the html and added a png result so developers can see if the test passes or not.
> > What part of this result represents the correctness of this patch? > Kent-san, I modified the html and added a png result so developers can see if > the test passes or not. ok, the updated test result looks good.
Comment on attachment 48787 [details] bold_italic_font_face_v2 The actual code change in this patch is somewhat mysterious to me, but according to the comments on this bug, this patch brings our behavior closer to Firefox and the specification. Yusuke has also addressed comments from mitz. Please wait a few days before landing this patch to give mitz and others a chance to object to this patch. @reviewers: If you'd rather patches like this one got more expert review, please be more agressive at r-ing or r+ing them. I'd rather not review this far outside my area, but we also don't want patches waiting for review forever.
Comment on attachment 48787 [details] bold_italic_font_face_v2 Dan says, "If we’re making this change, shouldn’t we entirely drop support for 'all' and for multiple values in @font-face descriptors?" I'm a bit confused by this change. Is it supported by the latest spec draft? Clearing flag for discussion.
(In reply to comment #11) Thanks for the comment. 'all' is no longer supported by the latest draft. I'll modify the CSS parser and attach a new patch later.
Created attachment 112461 [details] Patch
Created attachment 112462 [details] test expectation1
Created attachment 112463 [details] test expectation2
(In reply to comment #13) > Created an attachment (id=112461) [details] > Patch I've revised Yusuke-san's patch so that font selection logic matches the current draft (http://www.w3.org/TR/2011/WD-css3-fonts-20111004/#font-matching-algorithm). Attached expectations were generated by using chromium Linux port. Could you take another look?
Created attachment 112476 [details] Patch (revised Skipped and test_expectations.txt)
Comment on attachment 112476 [details] Patch (revised Skipped and test_expectations.txt) View in context: https://bugs.webkit.org/attachment.cgi?id=112476&action=review > Source/WebCore/css/CSSFontSelector.cpp:120 > + if (styleList->length() != 1) Does it make sense for this to ever be anything other than a single CSSPrimitiveValue here? I would remove handling of lists and remove generation of lists from the parser.
Created attachment 113279 [details] Patch
Comment on attachment 112476 [details] Patch (revised Skipped and test_expectations.txt) View in context: https://bugs.webkit.org/attachment.cgi?id=112476&action=review Thanks for the comments! >> Source/WebCore/css/CSSFontSelector.cpp:120 >> + if (styleList->length() != 1) > > Does it make sense for this to ever be anything other than a single CSSPrimitiveValue here? I would remove handling of lists and remove generation of lists from the parser. Done.
Comment on attachment 113279 [details] Patch Attachment 113279 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10258166 New failing tests: svg/W3C-SVG-1.1/fonts-desc-02-t.svg
Created attachment 113822 [details] Patch
(In reply to comment #22) > Created an attachment (id=113822) [details] > Patch Seems like we need to leave font-variant related code in font selection algorithm for CSS2.1 compatibility.
Created attachment 116856 [details] Patch
(In reply to comment #24) > Created an attachment (id=116856) [details] > Patch Ping, mitz?
Created attachment 121407 [details] Patch(revised to ToT)
(In reply to comment #26) > Created an attachment (id=121407) [details] > Patch(revised to ToT) Hi mitz, Could you please review this patch? If you have no time to review the patch, could you suggest another reviewers? Thanks,
Comment on attachment 121407 [details] Patch(revised to ToT) Attachment 121407 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11153006
Comment on attachment 121407 [details] Patch(revised to ToT) Attachment 121407 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11141294
Created attachment 121409 [details] Patch(fix compile failure)
Created attachment 126189 [details] Patch(just rebased)
Mitz and other reviewers, Could you please review the patch? We have been received several bug report related to this problem. - http://crbug.com/31833 (the original bug report) - http://crbug.com/101577 - http://code.google.com/p/chromium/issues/detail?id=39017#c66 - https://groups.google.com/a/googleproductforums.com/d/topic/chrome-ja/dRcBObfLh1c/discussion (in Japanese)
Comment on attachment 126189 [details] Patch(just rebased) View in context: https://bugs.webkit.org/attachment.cgi?id=126189&action=review > Source/WebCore/css/CSSFontSelector.cpp:432 > + // - If the desired weight is 500, 400 is checked first and then the rule for desired weights less than 400 is used. This almost seems like a typo in the spec. If you specify 500, you get 100 before you get 600.
Comment on attachment 126189 [details] Patch(just rebased) View in context: https://bugs.webkit.org/attachment.cgi?id=126189&action=review Hi mitz, Thank you so much for review. >> Source/WebCore/css/CSSFontSelector.cpp:432 >> + // - If the desired weight is 500, 400 is checked first and then the rule for desired weights less than 400 is used. > > This almost seems like a typo in the spec. If you specify 500, you get 100 before you get 600. I'm not sure whether it is a typo. I'd like to keep it at this time. I'll change the behavior if the spec is wrong.
Created attachment 126688 [details] Patch for landing
Comment on attachment 126688 [details] Patch for landing Clearing flags on attachment: 126688 Committed r107516: <http://trac.webkit.org/changeset/107516>
All reviewed patches have been landed. Closing bug.
This broke fast/css/font-face-default-font.html. See bug 79260.