Bug 49759

Summary: [Qt] Zero-sized font does not yet work
Product: WebKit Reporter: Darin Adler <darin>
Component: TextAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: alex, benjamin, commit-queue, darin, hausmann, hyatt, jamesr, kling, ossy, robert
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 49879    
Bug Blocks:    
Attachments:
Description Flags
Patch none

Darin Adler
Reported 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.
Attachments
Patch (8.28 KB, patch)
2010-11-22 14:07 PST, Robert Hogan
no flags
James Robinson
Comment 1 2010-11-18 15:43:26 PST
A consequence of this is that Qt almost certainly fails acid3 currently.
Alejandro G. Castro
Comment 2 2010-11-19 04:46:17 PST
It also fails in GTK, skipping the test to check the issue.
Robert Hogan
Comment 3 2010-11-20 08:25:10 PST
void QFont::setPixelSize(int pixelSize) { if (pixelSize <= 0) { qWarning("QFont::setPixelSize: Pixel size <= 0 (%d)", pixelSize); return; }
Andreas Kling
Comment 4 2010-11-20 08:30:28 PST
*** Bug 49851 has been marked as a duplicate of this bug. ***
Robert Hogan
Comment 5 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.
Robert Hogan
Comment 6 2010-11-21 10:25:47 PST
Robert Hogan
Comment 7 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);
Robert Hogan
Comment 8 2010-11-22 14:07:45 PST
Robert Hogan
Comment 9 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.
Robert Hogan
Comment 10 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?
Robert Hogan
Comment 11 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!
Robert Hogan
Comment 12 2010-11-29 13:46:36 PST
Andreas, can you have a look at this?
Andreas Kling
Comment 13 2010-11-30 03:08:19 PST
Comment on attachment 74595 [details] Patch LGTM, thanks for fixing this Robert!
WebKit Commit Bot
Comment 14 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>
WebKit Commit Bot
Comment 15 2010-12-05 07:10:43 PST
All reviewed patches have been landed. Closing bug.
Robert Hogan
Comment 16 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
Csaba Osztrogonác
Comment 17 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
Robert Hogan
Comment 18 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.
Csaba Osztrogonác
Comment 19 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.
Note You need to log in before you can comment on or make changes to this bug.