Bug 29433 – [Qt] @font-face rule does not work in QtWebKit but works in Safari

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Tor Arne Vestbø]

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)

[Andri Möll]

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.

[Strahinja Markovic]

Is there any chance whatsoever that this will be fixed for Qt 4.6.1?

[Strahinja Markovic]

I'm referring to the font weights issue.

[Kent Hansen]

How about a small testcase?

[Strahinja Markovic]

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.

[Strahinja Markovic]

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.

[Kent Hansen]

Reproduced on Linux against r55658, Qt 4.6.3.
Thanks for the testcase!

[Strahinja Markovic]

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.

[Antonio Gomes]

(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.

[Simon Hausmann]

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.

[Strahinja Markovic]

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.

[Henry Haverinen]

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. :-)

[Jocelyn Turcotte]

Related: Bug #36351

[Robert Hogan]

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); }

[Robert Hogan]

(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!

[Robert Hogan]

Created attachment 73275 [details]
Patch

[Robert Hogan]

(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.)

[Robert Hogan]

Comment on attachment 73275 [details]
Patch

This isn't right. Getting close though..

[Robert Hogan]

Created attachment 73420 [details]
Patch

[Robert Hogan]

(In reply to comment #19)
> Created an attachment (id=73420) [details]
> Patch

I'm pretty sure I'm

[Robert Hogan]

(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?

[Benjamin Poulain]

(In reply to comment #21)
> Benjamin, would you mind looking this over?

Will do.

[Benjamin Poulain]

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.

[Benjamin Poulain]

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

[Robert Hogan]

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.

[Benjamin Poulain]

(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.

[Robert Hogan]

(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.

[Benjamin Poulain]

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.

[Robert Hogan]

The sub-task in Qt bug tracker for this bugzilla entry is http://bugreports.qt.nokia.com/browse/QTBUG-15302

[Jiang Jiang]

(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.

[Pierre Rossi]

I think we can definitely close this one, since it works reasonably well, even with QFont.

[Kovid Goyal]

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.

[Pierre Rossi]

(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.

[Kovid Goyal]

Ah, thanks, I was hoping that was the case.