Bug 29433

Summary: [Qt] @font-face rule does not work in QtWebKit but works in Safari
Product: WebKit Reporter: Tor Arne Vestbø <vestbo>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: avatara.nsk, benjamin, gzjjgod, hausmann, henry.haverinen, jarred, jesus, jturcotte, kent.hansen, kovid, laszlo.gombos, peterjlai, pierre.rossi, robert, strahinja.markovic, tonikitoo
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: Other   
OS: All   
Bug Depends on:    
Bug Blocks: 29570, 63467    
Attachments:
Description Flags
Font variants test case
none
Patch
none
Patch none

Description Tor Arne Vestbø 2009-09-18 07:46:20 PDT
This bug report originated from issue QTBUG-3467
<http://bugreports.qt.nokia.com/browse/QTBUG-3467>

--- Description ---

The @font-face CSS rule rule works in Safari (which also uses WebKit), but seems not to be working under Qt (4.4 and 4.5 snapshots)
Comment 1 Andri Möll 2009-10-27 09:32:48 PDT
It actually does work with Qt 4.5.2/4.5.3, but it breaks when setting up different weights (@font-face font-weight) by using font files whose family names are identical and only the style attribute in the file changes. As a work-around one can edit each font in a font family and change its family name to be different.
Comment 2 Strahinja Markovic 2009-12-20 11:24:40 PST
Is there any chance whatsoever that this will be fixed for Qt 4.6.1?
Comment 3 Strahinja Markovic 2009-12-20 11:25:32 PST
I'm referring to the font weights issue.
Comment 4 Kent Hansen 2010-03-11 09:03:42 PST
How about a small testcase?
Comment 5 Strahinja Markovic 2010-03-11 11:32:15 PST
I created a test case and uploaded it, but it seems it didn't survive your transition from one bug tracker to the next.

I'll try to dig it out. It's still somewhere on my system.
Comment 6 Strahinja Markovic 2010-03-11 11:52:33 PST
Created attachment 50522 [details]
Font variants test case

Here is a new test case for the problem. It includes both an OTF normal/italic pair and a TTF one. All the fonts are OSS/free.

The lines with the word "italic" in them should display italic, but are instead displayed normally. The test case displays fine in Firefox 3.5+. I also tried it in Safari the first time I submitted a test case for this, and it worked there too.
Comment 7 Kent Hansen 2010-03-12 02:41:05 PST
Reproduced on Linux against r55658, Qt 4.6.3.
Thanks for the testcase!
Comment 8 Strahinja Markovic 2010-04-15 15:53:26 PDT
Is there any chance of this bug being fixed for QtWebkit 2.0? It's really killing QtWebkit as a WYSIWYG editor component, and with the recent push towards @font-face for websites, it's also important for general browsing.
Comment 9 Antonio Gomes 2010-04-16 08:57:08 PDT
(In reply to comment #8)
> Is there any chance of this bug being fixed for QtWebkit 2.0? It's really
> killing QtWebkit as a WYSIWYG editor component, and with the recent push
> towards @font-face for websites, it's also important for general browsing.

simon, henry, how important is that bug qtwebkit 2.0? If you say it is a good thing to have or high priority, I can work on that.
Comment 10 Simon Hausmann 2010-04-16 13:57:51 PDT
Currently this bug is not considered critical to the release. In contrast to that we currently do have 50 outstanding bugs blocking the release.

As an example bug 36755 is a blocker and it is cross-platform.
Comment 11 Strahinja Markovic 2010-04-20 15:36:05 PDT
I understand it may not be critical, but this bug has been outstanding for *fifteen* months now. It was reported on the Qt tracker back in Jan '09. It will never be considered critical since it's not a critical issue, just an important one (IMO). 

That doesn't mean it should stay broken for another six months.

Anyway, I can't tell you what to prioritize, I can only say that I think it's about time this bug is fixed. Anyone building a WYSIWYG editor with Qt and looking for something richer than QTextEdit (which includes a lot of people) will hit this bug eventually. If it weren't for this bug, Qt would be the *perfect* platform for powerful WYSIWYG editors.

And this is ignoring the elephant in the room: @font-face is taking off for web design. Since Opera 10, Firefox 3.5 and Safari 3.1, *every* major browser is now supporting @font-face (IE has had for a while, too). Even special font-serving CDNs have recently sprung up (see http://typekit.com/). Bottom line, it's important.

QtWebKit breaks this, even though vanilla WebKit has had support for it for many months now. It's a glaring omission, and IMO should have been fixed a long time ago.

But that's just me.
Comment 12 Henry Haverinen 2010-04-21 02:10:43 PDT
The Nokia QA team has plans to test the CSS font support in QtWebKit, see here:
http://bugreports.qt.nokia.com/browse/QTWEBKIT-103

This testing work has been planned for the QtWebKit 2.1 release. I hope we can prioritize bug fixing for this a bit higher, too. We do care about this feature. :-)
Comment 13 Jocelyn Turcotte 2010-09-27 07:56:40 PDT
Related: Bug #36351
Comment 14 Robert Hogan 2010-11-08 13:34:21 PST
Your test case works properly if you do:


@font-face { font-family: "Fontin"; src: url(file://./Fontin-Regular.ttf); }
@font-face { font-family: "Fontin"; font-style: italic; src: url(file://./Fontin-Italic.ttf); }
Comment 15 Robert Hogan 2010-11-08 13:35:01 PST
(In reply to comment #14)
> Your test case works properly if you do:
> 
> 
> @font-face { font-family: "Fontin"; src: url(file://./Fontin-Regular.ttf); }
> @font-face { font-family: "Fontin"; font-style: italic; src: url(file://./Fontin-Italic.ttf); }

That's cruft - you can ignore it!
Comment 16 Robert Hogan 2010-11-08 13:47:25 PST
Created attachment 73275 [details]
Patch
Comment 17 Robert Hogan 2010-11-08 13:59:13 PST
(In reply to comment #16)
> Created an attachment (id=73275) [details]
> Patch

The fonts are getting loaded correctly in CSSFontSelector and co. And we are loading the correct font in FontQt. For example if you try something like:

@font-face { font-family: "Fontin"; src: url(Fontin-Regular.ttf); }
@font-face { font-family: "Fontin"; font-style: italic; src: url(FreeSans.ttf); }
.fontTTF { font-family: "Fontin" }
<p class="fontTTF" ><i>Italic TTF text.</i></p>

You will get the FreeSans font, though unitalicized. (Italicized with the patch obviously).

However all of this good work is undone in FontQT when we cast it to a QFont. We need to look at the font's m_fontDescription and apply the properties specified there - otherwise QFont will not honour them. I've applied italic and bold in the patch:

@ -350,6 +351,8 @@ bool Font::canReturnFallbackFontsForComplexText()
 QFont Font::font() const
 {
     QFont f = primaryFont()->getQtFont();
+    f.setWeight(FontPlatformData::toQFontWeight(weight()));
+    f.setItalic(italic());
     if (m_letterSpacing != 0)
         f.setLetterSpacing(QFont::AbsoluteSpacing, m_letterSpacing);
     if (m_wordSpacing != 0)

but it seems to me we could also be doing:

    font.setFamily(familyName);
    font.setPixelSize(qRound(description.computedSize()));
    const bool smallCaps = description.smallCaps();
    font.setCapitalization(smallCaps ? QFont::SmallCaps : QFont::MixedCase);

I would be surprised if this didn't start us passing more tests in LayoutTests/CSS*.

One thing that puzzles me: in the case of the provided example I would have thought that Fontin-Italic.ttf (which is italicized already) would not need to be setItalic(true), even when cast to a QFont. Isn't that inherent in the data already? Or is an Italic font TTF just a regular TTF with a specific property specified in the ttf file?

(At first I thought we were loading the regular font and italicizing it, but my first example above shows this is not the case.)
Comment 18 Robert Hogan 2010-11-08 14:23:44 PST
Comment on attachment 73275 [details]
Patch

This isn't right. Getting close though..
Comment 19 Robert Hogan 2010-11-09 14:36:02 PST
Created attachment 73420 [details]
Patch
Comment 20 Robert Hogan 2010-11-09 14:41:48 PST
(In reply to comment #19)
> Created an attachment (id=73420) [details]
> Patch

I'm pretty sure I'm
Comment 21 Robert Hogan 2010-11-09 14:42:42 PST
(In reply to comment #20)
> (In reply to comment #19)
> > Created an attachment (id=73420) [details] [details]
> > Patch
> 
> I'm pretty sure I'm

learning not to add people to the cc list while typing a comment. :-)

Benjamin, would you mind looking this over?
Comment 22 Benjamin Poulain 2010-11-10 02:16:31 PST
(In reply to comment #21)
> Benjamin, would you mind looking this over?

Will do.
Comment 23 Benjamin Poulain 2010-11-10 02:43:12 PST
After looking at the problem and the patch I am rather convinced the problem is really missing API in Qt.

There are a quite a few part of the specification we do not follow at all (http://www.w3.org/TR/css3-fonts/#font-resources ):
-"Downloaded fonts are only available to documents that reference them, they should not be made available to other applications or other documents."
-"When font matching is done fonts defined using these rules are considered before other available fonts on a system." && "If the font family name is the same as a font family available in a given user's environment, it effectively hides the underlying font for documents that use the stylesheet."

Given the spec, I think the patch just workaround the reported bug but we avoid the whole problem altogether. I am more in favor of extending Qt fonts API, and then fix the problem here.
Comment 24 Benjamin Poulain 2010-11-10 04:41:54 PST
After discussing the matter with Jiang, I created a bug report on the Qt bug tracker: http://bugreports.qt.nokia.com/browse/QTBUG-15211

He is confident this can be implemented. He suggested for example to have a local QFontDatabase for a page and add fonts to it. Suggestions on what the API could look like are welcome on http://bugreports.qt.nokia.com/browse/QTBUG-15211
Comment 25 Robert Hogan 2010-11-10 10:57:46 PST
Thanks for looking it over so promptly!

(In reply to comment #23)
> After looking at the problem and the patch I am rather convinced the problem is really missing API in Qt.
> 
Agreed. For this specific bug we need API that allows us to pass binary font data to QFontDatabase and query that data for its family and styles.

I will add that to the wishlist on the Qt bug you opened.

> There are a quite a few part of the specification we do not follow at all (http://www.w3.org/TR/css3-fonts/#font-resources ):
> -"Downloaded fonts are only available to documents that reference them, they should not be made available to other applications or other documents."
> -"When font matching is done fonts defined using these rules are considered before other available fonts on a system." && "If the font family name is the same as a font family available in a given user's environment, it effectively hides the underlying font for documents that use the stylesheet."
> 
As you point out, using a page-specific QFontDatabase will solve both of these. But they are separate from this bug because even with a page-specific QFontDatabase we still won't know what new families or what new styles in a family a given blob of font data added to the database. Mind if I open a separate bug for them?

> Given the spec, I think the patch just workaround the reported bug but we avoid the whole problem altogether. I am more in favor of extending Qt fonts API, and then fix the problem here.

I'm not sure if my comments above alter your thinking on this. It would be nice to fix this particular bug now rather than wait for better API to fix a wider problem.
Comment 26 Benjamin Poulain 2010-11-10 12:50:32 PST
(In reply to comment #25)
> As you point out, using a page-specific QFontDatabase will solve both of these. But they are separate from this bug because even with a page-specific QFontDatabase we still won't know what new families or what new styles in a family a given blob of font data added to the database. Mind if I open a separate bug for them?

Good idea. We should have a bug mirroring QTBUG-15211 to do the implementation in QtWebKit.


> > Given the spec, I think the patch just workaround the reported bug but we avoid the whole problem altogether. I am more in favor of extending Qt fonts API, and then fix the problem here.
> 
> I'm not sure if my comments above alter your thinking on this. It would be nice to fix this particular bug now rather than wait for better API to fix a wider problem.

I would prefer new APIs to solve the problem in a more elegant way. I am really not a fan of querying the database to extract all the families. I also wonder if that could have any performance impact (fontconfig being a rather slow beast) or impact on memory usage.
I am tempted to just say we cannot support font-faces until QTBUG-15211 is fixed.

You know better than me about the current problem of bold and italic faces. If you think this cannot/shouldnot be solved by new APIs, I'll dig further into this patch.
Comment 27 Robert Hogan 2010-11-11 14:28:55 PST
(In reply to comment #26)
> I would prefer new APIs to solve the problem in a more elegant way. I am really not a fan of querying the database to extract all the families. I also wonder if that could have any performance impact (fontconfig being a rather slow beast) or impact on memory usage.

That's a good point. I'm sure it's pretty nasty.

> I am tempted to just say we cannot support font-faces until QTBUG-15211 is fixed.
Yeah, agreed. We need a way of doing this properly now that we know what needs to be done.
Comment 28 Benjamin Poulain 2010-11-12 07:13:27 PST
Comment on attachment 73420 [details]
Patch

Clearing the flag for now. We need to design the new API soon and then decide if we still need this workaround.
Comment 29 Robert Hogan 2010-11-12 10:50:32 PST
The sub-task in Qt bug tracker for this bugzilla entry is http://bugreports.qt.nokia.com/browse/QTBUG-15302
Comment 30 Jiang Jiang 2010-12-08 06:33:04 PST
(In reply to comment #28)
> (From update of attachment 73420 [details])
> Clearing the flag for now. We need to design the new API soon and then decide if we still need this workaround.

Check http://bugreports.qt.nokia.com/browse/QTBUG-15211 for experimental patch for Mac OS X and a sample using the new API. Work has to be done for other systems in a similar fashion.
Comment 31 Pierre Rossi 2012-04-13 06:54:04 PDT
I think we can definitely close this one, since it works reasonably well, even with QFont.
Comment 32 Kovid Goyal 2012-07-10 04:10:51 PDT
Why has this bug been closed as fixed? It is not fixed. The attached test case still does not render the italic text as italic with Qt 4.8.2.
Comment 33 Pierre Rossi 2012-07-10 04:26:35 PDT
(In reply to comment #32)
> Why has this bug been closed as fixed? It is not fixed. The attached test case still does not render the italic text as italic with Qt 4.8.2.

It is fixed in trunk WebKit. Qt 4.8.2 still comes shipped with QtWebKit 2.2, which was branched off from trunk before that.

See here for instructions on how to build:
http://trac.webkit.org/wiki/QtWebKit

You might also be interested in keeping an eye on the upcoming unofficial QtWebKit 2.3 branch for Qt 4 that was discussed on the webkit-qt mailing list.
Comment 34 Kovid Goyal 2012-07-10 04:45:03 PDT
Ah, thanks, I was hoping that was the case.