Version: (using KDE KDE 3.1.2) Installed from: Compiled From Sources Compiler: GCC 3.3 SuSE RPMs OS: Linux If the first font in a font-style css declaration isn't available on the system, then subsequent fonts are not parsed - the default Konqueror font is selected itself. Test case: <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd"> <html> <head> <title>Testing</title> <style type="text/css"> p { font-size:20px; font-family: "non existent font", "Times New Roman", times, serif; color:black; background-color:white; } </style> </head> <body> <p>Hello world.</p> </body> </html> The web page above should display the text in Times (new roman) or a serif font. But, on my system, the default Konqueror font is a sans-serif font, and "Hello world" is displayed in this font, not the correct serifed font. In kcmkonqueror the sans-serif font is set to Bitsteam Vera Sans, the serif font to Bitstream Vera Serif, but I also have Times New Roman installed. This also affects external stylesheets, as well as inline ones. The above example works fine in Mozilla.
Sorry, should be font-family css attribute.
Correcting the title accordingly
*** Bug 60976 has been marked as a duplicate of this bug. ***
I too have encountered this bug, and I think I have discovered the root cause. (Source path and line numbers are from the kdelibs-3.1-10.src.rpm package distributed with Red Hat Linux 9.) The code for font family selection appears in khtml/css/cssstyleselector.cpp lines 2432-2474. That code correctly includes a loop over the stylesheet's font-family value, but it handles the "not found" case incorrectly. QString face = static_cast<FontFamilyValueImpl *>(val)->fontName(); if ( !face.isNull() || face.isEmpty() ) { if(face == "serif") { face = settings->serifFontName(); } else if(face == "sans-serif") { face = settings->sansSerifFontName(); } else if( face == "cursive") { face = settings->cursiveFontName(); } else if( face == "fantasy") { face = settings->fantasyFontName(); } else if( face == "monospace") { face = settings->fixedFontName(); } else if( face == "konq_default") { face = settings->stdFontName(); } if ( !face.isEmpty() ) { fontDef.family = face; if (style->setFontDef( fontDef )) fontDirty = true; } return; } The goal is apparently to look up the font-family in the Qt font database and set the font-family if there's a match. However, the "if" condition is incorrect: It's always true, even if there's no match! Because of this, the first font-family always "matches"; if the font doesn't actually exist, Konqueror apparently uses its default font. It looks like there's a set of parentheses missing. I think the following code would be correct: if ( ! (face.isNull() || face.isEmpty()) ) { However, I think the test can be simplified further. All Null QStrings are also Empty. Therefore, the following test should be equivalent: if ( !face.isEmpty() ) { Unfortunately, I don't have a good test system to test this change. The error is simple, though, and this one-line change should be sufficient to fix it, unless I'm overlooking something. Hopefully, the KDE developers can fix it easily. Note that I also discovered another possible bug while investigating this. At first, I thought this might be an Xft bug. I tried creating an alias for the missing font, but that made no difference. Qt's font database does not include aliases in the db.families() call (used by KHTMLSettings::AvailableFamilies() and indirectly by the CSS routines). Therefore, a user cannot supply aliases for fonts used in font-family declarations, which is a potential problem for Linux users who have URW Palladio installed instead of a true Palatino font (for example). I don't know whether there's a way to fix this using Qt; a brief skim of the manpages didn't turn anything up. Therefore, this problem might be a Qt bug rather than a KHTML bug.
Subject: Re: [PATCH] Konqueror doesn't parse font-family CSS correctly if first font doesn't exist [testcase] Thanks Bradd for your detective work and clear description of the problem! The following patch against current KDE_3_1_BRANCH fixes the problem for me: If anyone with CVS privileges could apply the patch, or something very similar this bug can be closed. I don't know if it the problem exists in HEAD as I'm not running it, and the code seems to have changed quite a bit. Jason Index: cssstyleselector.cpp =================================================================== RCS file: /home/kde/kdelibs/khtml/css/cssstyleselector.cpp,v retrieving revision 1.242.2.4 diff -u -3 -p -r1.242.2.4 cssstyleselector.cpp --- cssstyleselector.cpp 30 Apr 2003 21:41:07 -0000 1.242.2.4 +++ cssstyleselector.cpp 5 Aug 2003 08:13:00 -0000 @@ -2451,7 +2451,8 @@ void CSSStyleSelector::applyRule( DOM::C CSSPrimitiveValueImpl *val = static_cast<CSSPrimitiveValueImpl *>(item); if(!val->primitiveType() == CSSPrimitiveValue::CSS_STRING) return; QString face = static_cast<FontFamilyValueImpl *>(val)->fontName(); - if ( !face.isNull() || face.isEmpty() ) { + + if ( !face.isEmpty() ) { if(face == "serif") { face = settings->serifFontName(); }
unfortunately the code in HEAD looks slightly different. But I attach a test case that breaks.
Created attachment 2858 [details] test case should display in courier, displays in default sans font
sent patch to kfm-devel for review
Subject: kdelibs/khtml CVS commit by coolo: do not break on non existant fonts in css font lists CCMAIL: 60556-done@bugs.kde.org M +5 -0 ChangeLog 1.44 M +3 -2 css/cssstyleselector.cpp 1.294 --- kdelibs/khtml/ChangeLog #1.43:1.44 @@ -1,2 +1,7 @@ +2003-10-23 Stephan Kulow <coolo@kde.org> + + * css/cssstyleselector.cpp (applyRule): if the first font isn't found, + continue looking (#60556) + 2003-10-22 Dirk Mueller <mueller@kde.org> --- kdelibs/khtml/css/cssstyleselector.cpp #1.293:1.294 @@ -2928,8 +2928,9 @@ void CSSStyleSelector::applyRule( int id if ( !face.isEmpty() ) { fontDef.family = face; - if (style->setFontDef( fontDef )) + if (style->setFontDef( fontDef )) { fontDirty = true; - } return; + } + } } break;
This change created bug #67768
Subject: kdelibs/khtml CVS commit by coolo: slightly modified patch by Paul Sprakes to fix his own bug report which turned out to be a regression introduced by my fix for #60556 (see #67768 for detailed descriptions) CCMAIL: 60556@bugs.kde.org CCMAIL: 67768-done@bugs.kde.org CCMAIL: 67973-done@bugs.kde.org (will work out regression tests now :) M +6 -0 ChangeLog 1.102 M +7 -14 css/cssstyleselector.cpp 1.301 --- kdelibs/khtml/ChangeLog #1.101:1.102 @@ -1,2 +1,8 @@ +2003-11-23 Stephan Kulow <coolo@kde.org> + + * css/cssstyleselector.cpp (applyRule): fixing my fix for #60556, which + introduced quite some regressions (thanks to Paul Sprakes for working + that out with me) + 2003-11-22 Stephan Kulow <coolo@kde.org> --- kdelibs/khtml/css/cssstyleselector.cpp #1.300:1.301 @@ -1852,6 +1852,5 @@ void CSSStyleSelector::applyRule( int id } } - if (style->setFontDef( fontDef )) - fontDirty = true; + fontDirty |= style->setFontDef( fontDef ); break; } @@ -1874,6 +1873,5 @@ void CSSStyleSelector::applyRule( int id return; } - if (style->setFontDef( fontDef )) - fontDirty = true; + fontDirty |= style->setFontDef( fontDef ); break; } @@ -1918,6 +1916,5 @@ void CSSStyleSelector::applyRule( int id } } - if (style->setFontDef( fontDef )) - fontDirty = true; + fontDirty |= style->setFontDef( fontDef ); break; } @@ -2773,6 +2770,5 @@ void CSSStyleSelector::applyRule( int id fontDef.size = size; - if (style->setFontDef( fontDef )) - fontDirty = true; + fontDirty |= style->setFontDef( fontDef ); return; } @@ -2982,10 +2978,8 @@ void CSSStyleSelector::applyRule( int id if ( !face.isEmpty() ) { fontDef.family = face; - if (style->setFontDef( fontDef )) { - fontDirty = true; + fontDirty |= style->setFontDef( fontDef ); return; } } - } break; } @@ -3164,6 +3158,5 @@ void CSSStyleSelector::applyRule( int id FontDef fontDef = parentStyle->htmlFont().fontDef; style->setLineHeight( parentStyle->lineHeight() ); - if (style->setFontDef( fontDef )) - fontDirty = true; + fontDirty |= style->setFontDef( fontDef ); } else if ( value->isFontValue() ) { FontValueImpl *font = static_cast<FontValueImpl *>(value);