Bug 33299

Summary: [Gtk] FreeType backend does not respect FC_MATRIX property
Product: WebKit Reporter: Dino Morelli <dino>
Component: WebKitGTKAssignee: Martin Robinson <mrobinson>
Status: RESOLVED FIXED    
Severity: Major CC: a.butenka, alex, dbates, dino, mrobinson
Priority: P2 Keywords: PlatformOnly
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 36548    
Bug Blocks: 46216    
Attachments:
Description Flags
sample HTML document illustrating CSS italic issue
none
Patch for this issue cfleizach: review+

Description Dino Morelli 2010-01-06 21:05:33 PST
Created attachment 46018 [details]
sample HTML document illustrating CSS italic issue

Text that should display as italic is not rendering italic. Example HTML document attached.

This is happening in both the Midori and uzbl browsers on Arch Linux 32-bit with libwebkit 1.1.15.4
Comment 1 Dino Morelli 2010-01-06 21:17:03 PST
Versions of webkit browsers tested on the Arch Linux system:

midori 0.2.2-1
uzbl-git 20091205-1
Comment 2 Martin Robinson 2010-09-17 15:35:19 PDT
The issue here seems to be that we are totally ignoring the FC_MATRIX property of the FcPattern we use to create the font.
Comment 3 Martin Robinson 2010-09-17 15:36:41 PDT
The FC_MATRIX property is *very* lightly documented, but we can observe it in Pango.
Comment 4 Martin Robinson 2010-09-21 12:11:29 PDT
Created attachment 68273 [details]
Patch for this issue
Comment 5 Alexander Butenko 2010-09-28 10:08:58 PDT
ping?
Comment 6 Martin Robinson 2010-09-28 10:39:57 PDT
I'll see if I can find someone to review this today.
Comment 7 chris fleizach 2010-10-05 10:04:38 PDT
Comment on attachment 68273 [details]
Patch for this issue

View in context: https://bugs.webkit.org/attachment.cgi?id=68273&action=review

r=me, but you should address the "new SimpleFontData" thing, even though its existing code

> WebCore/platform/graphics/cairo/SimpleFontDataCairo.cpp:89
>  

shouldn't this be adoptRef or something. Naked news are no longer in favor i believe.
Comment 8 Martin Robinson 2010-10-05 10:33:28 PDT
(In reply to comment #7)

> shouldn't this be adoptRef or something. Naked news are no longer in favor i believe.

Thanks for the review! I don't know if adoptRef is appropriate here because SimpleFontData isn't a descendant of RefCounted. In particular, it seems like 
m_smallCapsFontData is owned only by SimpleFontData.
Comment 9 Martin Robinson 2010-10-05 11:51:36 PDT
(In reply to comment #8)

> Thanks for the review! I don't know if adoptRef is appropriate here because SimpleFontData isn't a descendant of RefCounted. In particular, it seems like  m_smallCapsFontData is owned only by SimpleFontData.

Chris, given this, do you still mind if I land the patch as-is?
Comment 10 Martin Robinson 2010-10-07 10:19:21 PDT
I'm almost 100% certain about the SimpleFontData issue, so I'll land as-is. If this is wrong though, we can roll it out ASAP.
Comment 11 Martin Robinson 2010-10-07 10:47:46 PDT
Committed r69320: <http://trac.webkit.org/changeset/69320>