Bug 49759 – [Qt] Zero-sized font does not yet work

• Status: RESOLVED
• Resolution: FIXED
• Component: Text

[Darin Adler]

Zero-sized fonts need to render as nothing and take no space. Hyatt got this working on other platforms but it seems that it is not yet working for Qt given that bot failures show fast/text/font-size-zero.html failing.

[James Robinson]

A consequence of this is that Qt almost certainly fails acid3 currently.

[Alejandro G. Castro]

It also fails in GTK, skipping the test to check the issue.

[Robert Hogan]

void QFont::setPixelSize(int pixelSize)
{
    if (pixelSize <= 0) {
        qWarning("QFont::setPixelSize: Pixel size <= 0 (%d)", pixelSize);
        return;
    }

[Andreas Kling]

*** Bug 49851 has been marked as a duplicate of this bug. ***

[Robert Hogan]

In addition to a Qt API change permitting zero pixel-size fonts we need to fix 49879 to allow this to pass.

[Robert Hogan]

See http://bugreports.qt.nokia.com/browse/QTBUG-15485

[Robert Hogan]

Note we also need to do this in DRT:


    globalSettings->setFontSize(QWebSettings::MinimumFontSize, 0);
    globalSettings->setFontSize(QWebSettings::MinimumLogicalFontSize, 0);

[Robert Hogan]

Created attachment 74595 [details]
Patch

[Robert Hogan]

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

A Qt API change is undesirable according to the Qt Jira and gitorious comments and this can be fixed in QtWebKit anyway.

[Robert Hogan]

I asked Jiang about this and he said:

"I'm afraid there is no reliable way in Qt now to verify if a font family exists. Can you please create a bug report in http://bugreports.qt.nokia.com/ ? The workaround has to be platform specific."

See http://bugreports.qt.nokia.com/browse/QTBUG-15575

Should we fix it for now in QtWebKit using the attached patch?

[Robert Hogan]

(In reply to comment #10)
> I asked Jiang about this and he said:
> 
> "I'm afraid there is no reliable way in Qt now to verify if a font family exists. Can you please create a bug report in http://bugreports.qt.nokia.com/ ? The workaround has to be platform specific."
> 
> See http://bugreports.qt.nokia.com/browse/QTBUG-15575
> 
> Should we fix it for now in QtWebKit using the attached patch?

Whoops, wrong font bug!

[Robert Hogan]

Andreas, can you have a look at this?

[Andreas Kling]

Comment on attachment 74595 [details]
Patch

LGTM, thanks for fixing this Robert!

[WebKit Commit Bot]

Comment on attachment 74595 [details]
Patch

Clearing flags on attachment: 74595

Committed r73341: <http://trac.webkit.org/changeset/73341>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Robert Hogan]

Test has since been updated to test for font width so it still fails in qt.

Patch to fix the failure at https://bugs.webkit.org/show_bug.cgi?id=50539

[Csaba Osztrogonác]

(In reply to comment #16)
> Test has since been updated to test for font width so it still fails in qt.
> 
> Patch to fix the failure at https://bugs.webkit.org/show_bug.cgi?id=50539

Next time please don't leave the bot red for a day. :S

[Robert Hogan]

(In reply to comment #17)
> (In reply to comment #16)
> > Test has since been updated to test for font width so it still fails in qt.
> > 
> > Patch to fix the failure at https://bugs.webkit.org/show_bug.cgi?id=50539
> 
> Next time please don't leave the bot red for a day. :S

Yeah sorry, should I have just re-skipped the test while waiting for the follow-up review? I figured the shortest route to fixing was just to correct the issue with a follow-up patch.

[Csaba Osztrogonác]

(In reply to comment #18)
> Yeah sorry, should I have just re-skipped the test while waiting for the follow-up review? I figured the shortest route to fixing was just to correct the issue with a follow-up patch.

Not a big problem. But we like always green bots, because you can notice 
sooner a green vs. red change than a 1 vs. 2 failing tests change.