Changeset 107516 in webkit


Ignore:
Timestamp:
Feb 12, 2012 4:43:27 PM (12 years ago)
Author:
bashi@chromium.org
Message:

If @font-face does not provide an explicit italic/bold variant, regular is used.
​https://bugs.webkit.org/show_bug.cgi?id=34147

Reviewed by Dan Bernstein.

Source/WebCore:

Update @font-face handling code so that it matches @font-face behavior to the current draft of CSS3 Font spec. The original patch was written by ​yusukes@chromium.org.

  • Drops support for "bolder", "lighter", and "all" value. These are no longer allowed.
  • Only allows one value for font-style and font-weight.

Tests: fast/css/font-face-synthetic-bold-italic.html

fast/css/font-face-weight-matching.html

  • css/CSSFontSelector.cpp:

(WebCore::CSSFontSelector::addFontFaceRule): Removed "all", "lighter", "bolder" handling code.
(WebCore::compareFontFaces):Updated the weight matching algortihm.

  • css/CSSParser.cpp:

(WebCore::CSSParser::parseValue): Replaced parseFontStyle() call with checking primitive values.
(WebCore::CSSParser::parseFontWeight): Changed to allow only primitive values.
(WebCore::CSSParser::createFontFaceRule): Removed checks for font-weight and font-style.
(WebCore::CSSParser::deleteFontFaceOnlyValues): Ditto.

  • css/CSSParser.h: Removed parseFontStyle().

LayoutTests:

Added two tests to make sure font matching algorithm matches the current draft of CSS Fonts Module Level 3, and removed a test which uses obsoleted 'all' value for font-weight and font-style.

  • fast/css/font-face-descriptor-multiple-values-parsing-expected.txt: Removed.
  • fast/css/font-face-descriptor-multiple-values-parsing.html: Removed.
  • fast/css/font-face-synthetic-bold-italic.html: Added.
  • fast/css/font-face-weight-matching.html: Added.
  • platform/chromium/test_expectations.txt: Needs rebaselines.
Location:
trunk
Files:
2 added
2 deleted
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r107511 r107516  
     12012-02-12  Kenichi Ishibashi  <bashi@chromium.org>
     2
     3        If @font-face does not provide an explicit italic/bold variant, regular is used.
     4        https://bugs.webkit.org/show_bug.cgi?id=34147
     5
     6        Reviewed by Dan Bernstein.
     7
     8        Added two tests to make sure font matching algorithm matches the current draft of CSS Fonts Module Level 3, and removed a test which uses obsoleted 'all' value for font-weight and font-style.
     9
     10        * fast/css/font-face-descriptor-multiple-values-parsing-expected.txt: Removed.
     11        * fast/css/font-face-descriptor-multiple-values-parsing.html: Removed.
     12        * fast/css/font-face-synthetic-bold-italic.html: Added.
     13        * fast/css/font-face-weight-matching.html: Added.
     14        * platform/chromium/test_expectations.txt: Needs rebaselines.
     15
    1162012-02-12  Csaba Osztrogonác  <ossy@webkit.org>
    217

  • trunk/LayoutTests/platform/chromium/test_expectations.txt

    r107469 r107516  
    39793979BUGWK78367 : media/video-zoom-controls.html = IMAGE
    39803980BUGWK78367 : media/video-zoom.html = IMAGE
     3981
     3982BUGWK34147 : fast/css/font-face-synthetic-bold-italic.html = MISSING FAIL
     3983BUGWK34147 : fast/css/font-face-weight-matching.html = MISSING FAIL

  • trunk/Source/WebCore/ChangeLog

    r107515 r107516  
     12012-02-12  Kenichi Ishibashi  <bashi@chromium.org>
     2
     3        If @font-face does not provide an explicit italic/bold variant, regular is used.
     4        https://bugs.webkit.org/show_bug.cgi?id=34147
     5
     6        Reviewed by Dan Bernstein.
     7
     8        Update @font-face handling code so that it matches @font-face behavior to the current draft of CSS3 Font spec. The original patch was written by yusukes@chromium.org.
     9        - Drops support for "bolder", "lighter", and "all" value. These are no longer allowed.
     10        - Only allows one value for font-style and font-weight.
     11
     12        Tests: fast/css/font-face-synthetic-bold-italic.html
     13               fast/css/font-face-weight-matching.html
     14
     15        * css/CSSFontSelector.cpp:
     16        (WebCore::CSSFontSelector::addFontFaceRule): Removed "all", "lighter", "bolder" handling code.
     17        (WebCore::compareFontFaces):Updated the weight matching algortihm.
     18        * css/CSSParser.cpp:
     19        (WebCore::CSSParser::parseValue): Replaced parseFontStyle() call with checking primitive values.
     20        (WebCore::CSSParser::parseFontWeight): Changed to allow only primitive values.
     21        (WebCore::CSSParser::createFontFaceRule): Removed checks for font-weight and font-style.
     22        (WebCore::CSSParser::deleteFontFaceOnlyValues): Ditto.
     23        * css/CSSParser.h: Removed parseFontStyle().
     24
    1252012-02-12  David Reveman  <reveman@chromium.org>
    226

  • trunk/Source/WebCore/css/CSSFontSelector.cpp

    r106695 r107516  
    107107
    108108    if (RefPtr<CSSValue> fontStyle = style->getPropertyCSSValue(CSSPropertyFontStyle)) {
    109         if (fontStyle->isPrimitiveValue()) {
    110             RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
    111             list->append(fontStyle);
    112             fontStyle = list;
    113         } else if (!fontStyle->isValueList())
     109        if (!fontStyle->isPrimitiveValue())
    114110            return;
    115111
    116         CSSValueList* styleList = static_cast<CSSValueList*>(fontStyle.get());
    117         unsigned numStyles = styleList->length();
    118         if (!numStyles)
     112        switch (static_cast<CSSPrimitiveValue*>(fontStyle.get())->getIdent()) {
     113        case CSSValueNormal:
     114            traitsMask |= FontStyleNormalMask;
     115            break;
     116        case CSSValueItalic:
     117        case CSSValueOblique:
     118            traitsMask |= FontStyleItalicMask;
     119            break;
     120        default:
     121            break;
     122        }
     123    } else
     124        traitsMask |= FontStyleNormalMask;
     125
     126    if (RefPtr<CSSValue> fontWeight = style->getPropertyCSSValue(CSSPropertyFontWeight)) {
     127        if (!fontWeight->isPrimitiveValue())
    119128            return;
    120129
    121         for (unsigned i = 0; i < numStyles; ++i) {
    122             switch (static_cast<CSSPrimitiveValue*>(styleList->itemWithoutBoundsCheck(i))->getIdent()) {
    123                 case CSSValueAll:
    124                     traitsMask |= FontStyleMask;
    125                     break;
    126                 case CSSValueNormal:
    127                     traitsMask |= FontStyleNormalMask;
    128                     break;
    129                 case CSSValueItalic:
    130                 case CSSValueOblique:
    131                     traitsMask |= FontStyleItalicMask;
    132                     break;
    133                 default:
    134                     break;
    135             }
     130        switch (static_cast<CSSPrimitiveValue*>(fontWeight.get())->getIdent()) {
     131        case CSSValueBold:
     132        case CSSValue700:
     133            traitsMask |= FontWeight700Mask;
     134            break;
     135        case CSSValueNormal:
     136        case CSSValue400:
     137            traitsMask |= FontWeight400Mask;
     138            break;
     139        case CSSValue900:
     140            traitsMask |= FontWeight900Mask;
     141            break;
     142        case CSSValue800:
     143            traitsMask |= FontWeight800Mask;
     144            break;
     145        case CSSValue600:
     146            traitsMask |= FontWeight600Mask;
     147            break;
     148        case CSSValue500:
     149            traitsMask |= FontWeight500Mask;
     150            break;
     151        case CSSValue300:
     152            traitsMask |= FontWeight300Mask;
     153            break;
     154        case CSSValue200:
     155            traitsMask |= FontWeight200Mask;
     156            break;
     157        case CSSValue100:
     158            traitsMask |= FontWeight100Mask;
     159            break;
     160        default:
     161            break;
    136162        }
    137163    } else
    138         traitsMask |= FontStyleMask;
    139 
    140     if (RefPtr<CSSValue> fontWeight = style->getPropertyCSSValue(CSSPropertyFontWeight)) {
    141         if (fontWeight->isPrimitiveValue()) {
    142             RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
    143             list->append(fontWeight);
    144             fontWeight = list;
    145         } else if (!fontWeight->isValueList())
    146             return;
    147 
    148         CSSValueList* weightList = static_cast<CSSValueList*>(fontWeight.get());
    149         unsigned numWeights = weightList->length();
    150         if (!numWeights)
    151             return;
    152 
    153         for (unsigned i = 0; i < numWeights; ++i) {
    154             switch (static_cast<CSSPrimitiveValue*>(weightList->itemWithoutBoundsCheck(i))->getIdent()) {
    155                 case CSSValueAll:
    156                     traitsMask |= FontWeightMask;
    157                     break;
    158                 case CSSValueBolder:
    159                 case CSSValueBold:
    160                 case CSSValue700:
    161                     traitsMask |= FontWeight700Mask;
    162                     break;
    163                 case CSSValueNormal:
    164                 case CSSValue400:
    165                     traitsMask |= FontWeight400Mask;
    166                     break;
    167                 case CSSValue900:
    168                     traitsMask |= FontWeight900Mask;
    169                     break;
    170                 case CSSValue800:
    171                     traitsMask |= FontWeight800Mask;
    172                     break;
    173                 case CSSValue600:
    174                     traitsMask |= FontWeight600Mask;
    175                     break;
    176                 case CSSValue500:
    177                     traitsMask |= FontWeight500Mask;
    178                     break;
    179                 case CSSValue300:
    180                     traitsMask |= FontWeight300Mask;
    181                     break;
    182                 case CSSValueLighter:
    183                 case CSSValue200:
    184                     traitsMask |= FontWeight200Mask;
    185                     break;
    186                 case CSSValue100:
    187                     traitsMask |= FontWeight100Mask;
    188                     break;
    189                 default:
    190                     break;
    191             }
    192         }
    193     } else
    194         traitsMask |= FontWeightMask;
     164        traitsMask |= FontWeight400Mask;
    195165
    196166    if (RefPtr<CSSValue> fontVariant = style->getPropertyCSSValue(CSSPropertyFontVariant)) {
     167        // font-variant descriptor can be a value list.
    197168        if (fontVariant->isPrimitiveValue()) {
    198169            RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
    … …  
    209180        for (unsigned i = 0; i < numVariants; ++i) {
    210181            switch (static_cast<CSSPrimitiveValue*>(variantList->itemWithoutBoundsCheck(i))->getIdent()) {
    211                 case CSSValueAll:
    212                     traitsMask |= FontVariantMask;
    213                     break;
    214182                case CSSValueNormal:
    215183                    traitsMask |= FontVariantNormalMask;
    … …  
    428396        return firstHasDesiredVariant;
    429397
     398    // We need to check font-variant css property for CSS2.1 compatibility.
    430399    if ((desiredTraitsMaskForComparison & FontVariantSmallCapsMask) && !first->isLocalFallback() && !second->isLocalFallback()) {
    431400        // Prefer a font that has indicated that it can only support small-caps to a font that claims to support
    … …  
    457426        return true;
    458427
    459     // http://www.w3.org/TR/2002/WD-css3-webfonts-20020802/#q46 says: "If there are fewer then 9 weights in the family, the default algorithm
    460     // for filling the "holes" is as follows. If '500' is unassigned, it will be assigned the same font as '400'. If any of the values '600',
    461     // '700', '800', or '900' remains unassigned, they are assigned to the same face as the next darker assigned keyword, if any, or the next
    462     // lighter one otherwise. If any of '300', '200', or '100' remains unassigned, it is assigned to the next lighter assigned keyword, if any,
    463     // or the next darker otherwise."
    464     // For '400', we made up our own rule (which then '500' follows).
     428    // http://www.w3.org/TR/2011/WD-css3-fonts-20111004/#font-matching-algorithm says :
     429    //   - If the desired weight is less than 400, weights below the desired weight are checked in descending order followed by weights above the desired weight in ascending order until a match is found.
     430    //   - If the desired weight is greater than 500, weights above the desired weight are checked in ascending order followed by weights below the desired weight in descending order until a match is found.
     431    //   - If the desired weight is 400, 500 is checked first and then the rule for desired weights less than 400 is used.
     432    //   - If the desired weight is 500, 400 is checked first and then the rule for desired weights less than 400 is used.
    465433
    466434    static const unsigned fallbackRuleSets = 9;
    … …  
    470438        { FontWeight100Mask, FontWeight300Mask, FontWeight400Mask, FontWeight500Mask, FontWeight600Mask, FontWeight700Mask, FontWeight800Mask, FontWeight900Mask },
    471439        { FontWeight200Mask, FontWeight100Mask, FontWeight400Mask, FontWeight500Mask, FontWeight600Mask, FontWeight700Mask, FontWeight800Mask, FontWeight900Mask },
    472         { FontWeight500Mask, FontWeight300Mask, FontWeight600Mask, FontWeight200Mask, FontWeight700Mask, FontWeight100Mask, FontWeight800Mask, FontWeight900Mask },
    473         { FontWeight400Mask, FontWeight300Mask, FontWeight600Mask, FontWeight200Mask, FontWeight700Mask, FontWeight100Mask, FontWeight800Mask, FontWeight900Mask },
     440        { FontWeight500Mask, FontWeight300Mask, FontWeight200Mask, FontWeight100Mask, FontWeight600Mask, FontWeight700Mask, FontWeight800Mask, FontWeight900Mask },
     441        { FontWeight400Mask, FontWeight300Mask, FontWeight200Mask, FontWeight100Mask, FontWeight600Mask, FontWeight700Mask, FontWeight800Mask, FontWeight900Mask },
    474442        { FontWeight700Mask, FontWeight800Mask, FontWeight900Mask, FontWeight500Mask, FontWeight400Mask, FontWeight300Mask, FontWeight200Mask, FontWeight100Mask },
    475443        { FontWeight800Mask, FontWeight900Mask, FontWeight600Mask, FontWeight500Mask, FontWeight400Mask, FontWeight300Mask, FontWeight200Mask, FontWeight100Mask },

  • trunk/Source/WebCore/css/CSSParser.cpp

    r107369 r107516  
    14121412
    14131413    case CSSPropertyFontStyle:           // normal | italic | oblique | inherit
    1414         return parseFontStyle(important);
     1414        if (id == CSSValueNormal || id == CSSValueItalic || id == CSSValueOblique)
     1415            validPrimitive = true;
     1416        break;
    14151417
    14161418    case CSSPropertyFontVariant:         // normal | small-caps | inherit
    … …  
    43394341}
    43404342
    4341 bool CSSParser::parseFontStyle(bool important)
    4342 {
    4343     RefPtr<CSSValueList> values;
    4344     if (m_valueList->size() > 1)
    4345         values = CSSValueList::createCommaSeparated();
    4346     CSSParserValue* val;
    4347     bool expectComma = false;
    4348     while ((val = m_valueList->current())) {
    4349         RefPtr<CSSPrimitiveValue> parsedValue;
    4350         if (!expectComma) {
    4351             expectComma = true;
    4352             if (val->id == CSSValueNormal || val->id == CSSValueItalic || val->id == CSSValueOblique)
    4353                 parsedValue = cssValuePool()->createIdentifierValue(val->id);
    4354             else if (val->id == CSSValueAll && !values) {
    4355                 // 'all' is only allowed in @font-face and with no other values. Make a value list to
    4356                 // indicate that we are in the @font-face case.
    4357                 values = CSSValueList::createCommaSeparated();
    4358                 parsedValue = cssValuePool()->createIdentifierValue(val->id);
    4359             }
    4360         } else if (val->unit == CSSParserValue::Operator && val->iValue == ',') {
    4361             expectComma = false;
    4362             m_valueList->next();
    4363             continue;
    4364         }
    4365 
    4366         if (!parsedValue)
    4367             return false;
    4368 
    4369         m_valueList->next();
    4370 
    4371         if (values)
    4372             values->append(parsedValue.release());
    4373         else {
    4374             addProperty(CSSPropertyFontStyle, parsedValue.release(), important);
    4375             return true;
    4376         }
    4377     }
    4378 
    4379     if (values && values->length()) {
    4380         m_hasFontFaceOnlyValues = true;
    4381         addProperty(CSSPropertyFontStyle, values.release(), important);
    4382         return true;
    4383     }
    4384 
    4385     return false;
    4386 }
    4387 
    43884343bool CSSParser::parseFontVariant(bool important)
    43894344{
    … …  
    44354390bool CSSParser::parseFontWeight(bool important)
    44364391{
    4437     RefPtr<CSSValueList> values;
    4438     if (m_valueList->size() > 1)
    4439         values = CSSValueList::createCommaSeparated();
    4440     CSSParserValue* val;
    4441     bool expectComma = false;
    4442     while ((val = m_valueList->current())) {
    4443         RefPtr<CSSPrimitiveValue> parsedValue;
    4444         if (!expectComma) {
    4445             expectComma = true;
    4446             if (val->unit == CSSPrimitiveValue::CSS_IDENT) {
    4447                 if (val->id >= CSSValueNormal && val->id <= CSSValue900)
    4448                     parsedValue = cssValuePool()->createIdentifierValue(val->id);
    4449                 else if (val->id == CSSValueAll && !values) {
    4450                     // 'all' is only allowed in @font-face and with no other values. Make a value list to
    4451                     // indicate that we are in the @font-face case.
    4452                     values = CSSValueList::createCommaSeparated();
    4453                     parsedValue = cssValuePool()->createIdentifierValue(val->id);
    4454                 }
    4455             } else if (validUnit(val, FInteger | FNonNeg, false)) {
    4456                 int weight = static_cast<int>(val->fValue);
    4457                 if (!(weight % 100) && weight >= 100 && weight <= 900)
    4458                     parsedValue = cssValuePool()->createIdentifierValue(CSSValue100 + weight / 100 - 1);
    4459             }
    4460         } else if (val->unit == CSSParserValue::Operator && val->iValue == ',') {
    4461             expectComma = false;
    4462             m_valueList->next();
    4463             continue;
    4464         }
    4465 
    4466         if (!parsedValue)
    4467             return false;
    4468 
    4469         m_valueList->next();
    4470 
    4471         if (values)
    4472             values->append(parsedValue.release());
    4473         else {
    4474             addProperty(CSSPropertyFontWeight, parsedValue.release(), important);
    4475             return true;
    4476         }
    4477     }
    4478 
    4479     if (values && values->length()) {
    4480         m_hasFontFaceOnlyValues = true;
    4481         addProperty(CSSPropertyFontWeight, values.release(), important);
     4392    if (m_valueList->size() != 1)
     4393        return false;
     4394
     4395    CSSParserValue* value = m_valueList->current();
     4396    if ((value->id >= CSSValueNormal) && (value->id <= CSSValue900)) {
     4397        addProperty(CSSPropertyFontWeight, cssValuePool()->createIdentifierValue(value->id), important);
    44824398        return true;
    44834399    }
    4484 
     4400    if (validUnit(value, FInteger | FNonNeg, false)) {
     4401        int weight = static_cast<int>(value->fValue);
     4402        if (!(weight % 100) && weight >= 100 && weight <= 900)
     4403            addProperty(CSSPropertyFontWeight, cssValuePool()->createIdentifierValue(CSSValue100 + weight / 100 - 1), important);
     4404        return true;
     4405    }
    44854406    return false;
    44864407}
    … …  
    88578778        CSSProperty* property = m_parsedProperties[i];
    88588779        int id = property->id();
    8859         if ((id == CSSPropertyFontWeight || id == CSSPropertyFontStyle || id == CSSPropertyFontVariant) && property->value()->isPrimitiveValue()) {
     8780        if (id == CSSPropertyFontVariant && property->value()->isPrimitiveValue()) {
    88608781            RefPtr<CSSValue> value = property->m_value.release();
    88618782            property->m_value = CSSValueList::createCommaSeparated();
    … …  
    90028923        CSSProperty* property = m_parsedProperties[i];
    90038924        int id = property->id();
    9004         if ((id == CSSPropertyFontWeight || id == CSSPropertyFontStyle || id == CSSPropertyFontVariant) && property->value()->isValueList()) {
     8925        if (id == CSSPropertyFontVariant && property->value()->isValueList()) {
    90058926            delete property;
    90068927            deletedProperties++;

  • trunk/Source/WebCore/css/CSSParser.h

    r106756 r107516  
    157157    static bool fastParseColor(RGBA32&, const String&, bool strict);
    158158
    159     bool parseFontStyle(bool important);
    160159    bool parseFontVariant(bool important);
    161160    bool parseFontWeight(bool important);
Note: See TracChangeset for help on using the changeset viewer.