Bug 49759 - [Qt] Zero-sized font does not yet work
Summary: [Qt] Zero-sized font does not yet work
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 49851 (view as bug list)
Depends on: 49879
Blocks:
  Show dependency treegraph
 
Reported: 2010-11-18 13:40 PST by Darin Adler
Modified: 2010-12-06 10:37 PST (History)
10 users (show)

See Also:


Attachments
Patch (8.28 KB, patch)
2010-11-22 14:07 PST, Robert Hogan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2010-11-18 13:40:24 PST
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.
Comment 1 James Robinson 2010-11-18 15:43:26 PST
A consequence of this is that Qt almost certainly fails acid3 currently.
Comment 2 Alejandro G. Castro 2010-11-19 04:46:17 PST
It also fails in GTK, skipping the test to check the issue.
Comment 3 Robert Hogan 2010-11-20 08:25:10 PST
void QFont::setPixelSize(int pixelSize)
{
    if (pixelSize <= 0) {
        qWarning("QFont::setPixelSize: Pixel size <= 0 (%d)", pixelSize);
        return;
    }
Comment 4 Andreas Kling 2010-11-20 08:30:28 PST
*** Bug 49851 has been marked as a duplicate of this bug. ***
Comment 5 Robert Hogan 2010-11-21 07:23:09 PST
In addition to a Qt API change permitting zero pixel-size fonts we need to fix 49879 to allow this to pass.
Comment 6 Robert Hogan 2010-11-21 10:25:47 PST
See http://bugreports.qt.nokia.com/browse/QTBUG-15485
Comment 7 Robert Hogan 2010-11-21 10:33:50 PST
Note we also need to do this in DRT:


    globalSettings->setFontSize(QWebSettings::MinimumFontSize, 0);
    globalSettings->setFontSize(QWebSettings::MinimumLogicalFontSize, 0);
Comment 8 Robert Hogan 2010-11-22 14:07:45 PST
Created attachment 74595 [details]
Patch
Comment 9 Robert Hogan 2010-11-22 14:09:32 PST
(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.
Comment 10 Robert Hogan 2010-11-24 02:18:56 PST
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?
Comment 11 Robert Hogan 2010-11-24 02:21:42 PST
(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!
Comment 12 Robert Hogan 2010-11-29 13:46:36 PST
Andreas, can you have a look at this?
Comment 13 Andreas Kling 2010-11-30 03:08:19 PST
Comment on attachment 74595 [details]
Patch

LGTM, thanks for fixing this Robert!
Comment 14 WebKit Commit Bot 2010-12-05 07:10:36 PST
Comment on attachment 74595 [details]
Patch

Clearing flags on attachment: 74595

Committed r73341: <http://trac.webkit.org/changeset/73341>
Comment 15 WebKit Commit Bot 2010-12-05 07:10:43 PST
All reviewed patches have been landed.  Closing bug.
Comment 16 Robert Hogan 2010-12-05 09:46:47 PST
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
Comment 17 Csaba Osztrogonác 2010-12-06 02:28:23 PST
(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
Comment 18 Robert Hogan 2010-12-06 10:32:25 PST
(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.
Comment 19 Csaba Osztrogonác 2010-12-06 10:37:02 PST
(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.