Summary: | css layout rendered wrong | ||
---|---|---|---|
Product: | [Applications] konqueror | Reporter: | Gregor B. Rosenauer <gregor.rosenauer> |
Component: | khtml renderer | Assignee: | Konqueror Developers <konq-bugs> |
Status: | RESOLVED FIXED | ||
Severity: | normal | ||
Priority: | NOR | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
Platform: | unspecified | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | ||
Sentry Crash Report: | |||
Attachments: |
testcase
patch |
Description
Gregor B. Rosenauer
2006-12-25 12:55:57 UTC
This is the relevant part of the frame's source: <div style="position:absolute; top:260px; left:20px; width:580px;"><span style="color:#0087c0; font-weight:bold; font-size:14px;">Auf der Suche nach dem perfekten Geschenk?</span><br> Unser Geschenkfinder präsentiert Ihnen konkrete Vorschläge speziell für jeden einzelnen Ihrer Liebsten und Freunde zugeschnitten. <br> Los! Beantworten Sie die Fragen und finden Sie das perfekte Geschenk schnell und einfach!</div> Try using a smaller font. The webpage is constructed with a dependency on a specific font-size. I don't understand your reasoning - there is a style "font-size: 14px" which should just work, as the used font can be scaled to any size. This should not result in the rendering Konqueror does. For the record, I tried using smaller sizes, but to no avail as Konqueror will always scale the font to 14px as requested by the style. There is definitely something wrong with the way stylesheets are rendered as I noticed some layout-issues elsewhere, too (will report). btw, the form on the left side (shopping-ad) is missing completely (though I do not really need it I'd prefer to judge by myself,-). reopening as the bug is still there and I tried what was suggested... The problem is the line-height. It can vary depending on type of font. I use Arial and Konqueror renders the page fine as long as I don't zoom. You could also have Konqueror set to some default zoom-level? I can reproduce the problem here. From what I can see, it stems from the font shorthand which has conflicting priorities. i.e: ordinarily, some properties are computed first (e.g: font-size) in order to be able to compute dependant others (e.g: line-height)... but the font shorthand contains both kinds! For instance, in current page, we have: font:250%/1.4em Georgia,Serif; font-size: 12px; the 1.4em line-height computation is given extra (and bogus) priority as part of the shorthand, so it ends up at 1.4em of the 250% font-size, instead of 1.4em of the final 12px... a simple way to solve would be to give up on using the font shorthand internally and fragment it instead in its composing properties. Needs more testing. Created attachment 19041 [details]
testcase
apparently Gecko and Presto engines have adopted that same strategy of
fragmenting the shorthand (see cssRules output at bottom of testcase).
Created attachment 19042 [details]
patch
SVN commit 618828 by ggarand: stop handling the font shorthand as a whole: its composing properties have incompatible priorities. BUG: 139205 M +47 -35 cssparser.cpp --- trunk/KDE/kdelibs/khtml/css/cssparser.cpp #618827:618828 @@ -1729,7 +1729,8 @@ // kDebug(6080) << "parsing font property current=" << valueList->currentValue << endl; bool valid = true; Value *value = valueList->current(); - FontValueImpl *font = new FontValueImpl; + CSSValueListImpl* family = 0; + CSSPrimitiveValueImpl *style = 0, *variant = 0, *weight = 0, *size = 0, *lineHeight = 0; // optional font-style, font-variant and font-weight while ( value ) { // kDebug( 6080 ) << "got value " << value->id << " / " << (value->unit == CSSPrimitiveValue::CSS_STRING || @@ -1748,44 +1749,44 @@ inherit = true; } */ else if ( id == CSS_VAL_ITALIC || id == CSS_VAL_OBLIQUE ) { - if ( font->style ) + if ( style ) goto invalid; - font->style = new CSSPrimitiveValueImpl( id ); + style = new CSSPrimitiveValueImpl( id ); } else if ( id == CSS_VAL_SMALL_CAPS ) { - if ( font->variant ) + if ( variant ) goto invalid; - font->variant = new CSSPrimitiveValueImpl( id ); + variant = new CSSPrimitiveValueImpl( id ); } else if ( id >= CSS_VAL_BOLD && id <= CSS_VAL_LIGHTER ) { - if ( font->weight ) + if ( weight ) goto invalid; - font->weight = new CSSPrimitiveValueImpl( id ); + weight = new CSSPrimitiveValueImpl( id ); } else { valid = false; } - } else if ( !font->weight && validUnit( value, FInteger|FNonNeg, true ) ) { - int weight = (int)value->fValue; + } else if ( !weight && validUnit( value, FInteger|FNonNeg, true ) ) { + int w = (int)value->fValue; int val = 0; - if ( weight == 100 ) + if ( w == 100 ) val = CSS_VAL_100; - else if ( weight == 200 ) + else if ( w == 200 ) val = CSS_VAL_200; - else if ( weight == 300 ) + else if ( w == 300 ) val = CSS_VAL_300; - else if ( weight == 400 ) + else if ( w == 400 ) val = CSS_VAL_400; - else if ( weight == 500 ) + else if ( w == 500 ) val = CSS_VAL_500; - else if ( weight == 600 ) + else if ( w == 600 ) val = CSS_VAL_600; - else if ( weight == 700 ) + else if ( w == 700 ) val = CSS_VAL_700; - else if ( weight == 800 ) + else if ( w == 800 ) val = CSS_VAL_800; - else if ( weight == 900 ) + else if ( w == 900 ) val = CSS_VAL_900; if ( val ) - font->weight = new CSSPrimitiveValueImpl( val ); + weight = new CSSPrimitiveValueImpl( val ); else valid = false; } else { @@ -1799,24 +1800,24 @@ goto invalid; // set undefined values to default - if ( !font->style ) - font->style = new CSSPrimitiveValueImpl( CSS_VAL_NORMAL ); - if ( !font->variant ) - font->variant = new CSSPrimitiveValueImpl( CSS_VAL_NORMAL ); - if ( !font->weight ) - font->weight = new CSSPrimitiveValueImpl( CSS_VAL_NORMAL ); + if ( !style ) + style = new CSSPrimitiveValueImpl( CSS_VAL_NORMAL ); + if ( !variant ) + variant = new CSSPrimitiveValueImpl( CSS_VAL_NORMAL ); + if ( !weight ) + weight = new CSSPrimitiveValueImpl( CSS_VAL_NORMAL ); // kDebug( 6080 ) << " got style, variant and weight current=" << valueList->currentValue << endl; // now a font size _must_ come // <absolute-size> | <relative-size> | <length> | <percentage> | inherit if ( value->id >= CSS_VAL_XX_SMALL && value->id <= CSS_VAL_LARGER ) - font->size = new CSSPrimitiveValueImpl( value->id ); + size = new CSSPrimitiveValueImpl( value->id ); else if ( validUnit( value, FLength|FPercent, strict ) ) { - font->size = new CSSPrimitiveValueImpl( value->fValue, (CSSPrimitiveValue::UnitTypes) value->unit ); + size = new CSSPrimitiveValueImpl( value->fValue, (CSSPrimitiveValue::UnitTypes) value->unit ); } value = valueList->next(); - if ( !font->size || !value ) + if ( !size || !value ) goto invalid; // kDebug( 6080 ) << " got size" << endl; @@ -1829,7 +1830,7 @@ if ( value->id == CSS_VAL_NORMAL ) { // default value, nothing to do } else if ( validUnit( value, FNumber|FLength|FPercent, strict ) ) { - font->lineHeight = new CSSPrimitiveValueImpl( value->fValue, (CSSPrimitiveValue::UnitTypes) value->unit ); + lineHeight = new CSSPrimitiveValueImpl( value->fValue, (CSSPrimitiveValue::UnitTypes) value->unit ); } else { goto invalid; } @@ -1837,23 +1838,34 @@ if ( !value ) goto invalid; } - if ( !font->lineHeight ) - font->lineHeight = new CSSPrimitiveValueImpl( CSS_VAL_NORMAL ); + if ( !lineHeight ) + lineHeight = new CSSPrimitiveValueImpl( CSS_VAL_NORMAL ); // kDebug( 6080 ) << " got line height current=" << valueList->currentValue << endl; // font family must come now - font->family = parseFontFamily(); + family = parseFontFamily(); - if ( valueList->current() || !font->family ) + if ( valueList->current() || !family ) goto invalid; //kDebug( 6080 ) << " got family, parsing ok!" << endl; - addProperty( CSS_PROP_FONT, font, important ); + addProperty( CSS_PROP_FONT_FAMILY, family, important ); + addProperty( CSS_PROP_FONT_STYLE, style, important ); + addProperty( CSS_PROP_FONT_VARIANT, variant, important ); + addProperty( CSS_PROP_FONT_WEIGHT, weight, important ); + addProperty( CSS_PROP_FONT_SIZE, size, important ); + addProperty( CSS_PROP_LINE_HEIGHT, lineHeight, important ); return true; invalid: //kDebug(6080) << " -> invalid" << endl; - delete font; + delete family; + delete style; + delete variant; + delete weight; + delete size; + delete lineHeight; + return false; } |