Changeset 47229 in webkit


Ignore:
Timestamp:
Aug 13, 2009 1:56:46 PM (15 years ago)
Author:
eric@webkit.org
Message:

2009-08-13 Eric Seidel <​eric@webkit.org>

Reviewed by David Hyatt.

wrong font size when css font-family includes monospace
​https://bugs.webkit.org/show_bug.cgi?id=19161

Update test results now that only "font-family: monospace" is 13px.
All other combinations of font-family are 16px default size.

Add two tests:
font-family-fallback-reset: catches a bug in inheriting font-family fallback.
computed-style-font-family-monospace: tests this bug, that only "font-family: monospace" is 13px.

  • fast/css/getComputedStyle/computed-style-font-family-expected.txt:
  • fast/css/getComputedStyle/computed-style-font-family-monospace-expected.txt: Added.
  • fast/css/getComputedStyle/computed-style-font-family-monospace.html: Added.
  • fast/css/getComputedStyle/font-family-fallback-reset-expected.txt: Added.
  • fast/css/getComputedStyle/font-family-fallback-reset.html: Added.
  • fast/css/getComputedStyle/resources/computed-style-font-family-monospace.js: Added. (fontSizeForFamilies):
  • fast/css/getComputedStyle/resources/font-family-fallback-reset.js: Added.
  • fast/js/method-check.html:
  • platform/mac/css1/font_properties/font_family-expected.txt:
  • platform/mac/css2.1/t1503-c522-font-family-00-b-expected.txt:
  • platform/mac/fast/text/monospace-width-cache-expected.txt:

2009-08-13 Eric Seidel <​eric@webkit.org>

Reviewed by David Hyatt.

wrong font size when css font-family includes monospace
​https://bugs.webkit.org/show_bug.cgi?id=19161

Firefox only uses fixed-width default size for exactly "font-family: monospace;".
WebKit has historically used fixed-width default size any time a
font-family includes monospace in the fallback list.

This patch corrects WebKit's behavior to match Firefox.
I also had to fix a bug in WebKit's font-family fallback behavior where
child elements would inherit parts of their parents fallback lists.

This patch is mostly just replacing all cases where we used to check for:
fontDescription.genericFontFamily() == MonospaceFamily
with:
fontDescription.useFixedDefaultSize()

Tests: fast/css/getComputedStyle/computed-style-font-family-monospace.html

fast/css/getComputedStyle/font-family-fallback-reset.html

  • css/CSSStyleSelector.cpp: (WebCore::CSSStyleSelector::applyProperty):

Deploy useFixedDefaultSize(). Also fix the bug where child
FontDescriptions would carry part of the parent font-family fallback list.

(WebCore::CSSStyleSelector::checkForGenericFamilyChange):

It's no longer alright to just check genericFontFamily(),
we have to check to make sure the changed style has a matching useFixedDefaultSize().

  • platform/graphics/FontDescription.h: (WebCore::FontDescription::useFixedDefaultSize):

Only use the fixed default size if we have one font family and it is "monospace".
"-webkit-monospace" is the internal representation of the CSS identifier "monospace".

Location:
trunk
Files:
6 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r47214 r47229  
     12009-08-13  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by David Hyatt.
     4
     5        wrong font size when css font-family includes monospace
     6        https://bugs.webkit.org/show_bug.cgi?id=19161
     7
     8        Update test results now that only "font-family: monospace" is 13px.
     9        All other combinations of font-family are 16px default size.
     10
     11        Add two tests:
     12        font-family-fallback-reset: catches a bug in inheriting font-family fallback.
     13        computed-style-font-family-monospace: tests this bug, that only "font-family: monospace" is 13px.
     14
     15        * fast/css/getComputedStyle/computed-style-font-family-expected.txt:
     16        * fast/css/getComputedStyle/computed-style-font-family-monospace-expected.txt: Added.
     17        * fast/css/getComputedStyle/computed-style-font-family-monospace.html: Added.
     18        * fast/css/getComputedStyle/font-family-fallback-reset-expected.txt: Added.
     19        * fast/css/getComputedStyle/font-family-fallback-reset.html: Added.
     20        * fast/css/getComputedStyle/resources/computed-style-font-family-monospace.js: Added.
     21        (fontSizeForFamilies):
     22        * fast/css/getComputedStyle/resources/font-family-fallback-reset.js: Added.
     23        * fast/js/method-check.html:
     24        * platform/mac/css1/font_properties/font_family-expected.txt:
     25        * platform/mac/css2.1/t1503-c522-font-family-00-b-expected.txt:
     26        * platform/mac/fast/text/monospace-width-cache-expected.txt:
     27
    1282009-08-13  Adam Roben  <aroben@apple.com>
    229

  • trunk/LayoutTests/fast/css/getComputedStyle/computed-style-font-family-expected.txt

    r45408 r47229  
    22
    33font-family: monospace, 'Lucida Grande', sans-serif;
    4 font-size: 13px;
     4font-size: 16px;
    55font-style: normal;
    66font-variant: normal;

  • trunk/LayoutTests/platform/mac/css1/font_properties/font_family-expected.txt

    r25970 r47229  
    1 layer at (0,0) size 785x1301
     1layer at (0,0) size 785x1313
    22  RenderView at (0,0) size 785x600
    3 layer at (0,0) size 785x1301
    4   RenderBlock {HTML} at (0,0) size 785x1301
    5     RenderBody {BODY} at (8,8) size 769x1285 [bgcolor=#CCCCCC]
     3layer at (0,0) size 785x1313
     4  RenderBlock {HTML} at (0,0) size 785x1313
     5    RenderBody {BODY} at (8,8) size 769x1297 [bgcolor=#CCCCCC]
    66      RenderBlock {P} at (0,0) size 769x18
    77        RenderText {#text} at (0,0) size 355x18
    … …  
    5151        RenderText {#text} at (0,0) size 396x18
    5252          text run at (0,0) width 396: "This sentence should be in a sans-serif font, not cursive."
    53       RenderBlock {P} at (0,464) size 769x15
    54         RenderText {#text} at (0,0) size 440x15
    55           text run at (0,0) width 440: "This sentence should be in a monospace font, not serif."
    56       RenderBlock {HR} at (0,492) size 769x2 [border: (1px inset #000000)]
    57       RenderBlock {DIV} at (0,507) size 769x211
     53      RenderBlock {P} at (0,464) size 769x18
     54        RenderText {#text} at (0,0) size 550x18
     55          text run at (0,0) width 550: "This sentence should be in a monospace font, not serif."
     56      RenderBlock {HR} at (0,498) size 769x2 [border: (1px inset #000000)]
     57      RenderBlock {DIV} at (0,513) size 769x211
    5858        RenderBlock {P} at (0,0) size 769x30
    5959          RenderText {#text} at (0,0) size 768x30
    … …  
    7575          RenderText {#text} at (0,0) size 352x15
    7676            text run at (0,0) width 352: "This sentence should be in a monospace font."
    77       RenderTable {TABLE} at (0,731) size 769x554 [border: (1px outset #808080)]
    78         RenderTableSection {TBODY} at (1,1) size 767x552
     77      RenderTable {TABLE} at (0,737) size 769x560 [border: (1px outset #808080)]
     78        RenderTableSection {TBODY} at (1,1) size 767x558
    7979          RenderTableRow {TR} at (0,0) size 767x26
    8080            RenderTableCell {TD} at (0,0) size 767x26 [bgcolor=#C0C0C0] [border: (1px inset #808080)] [r=0 c=0 rs=1 cs=2]
    … …  
    8282                RenderText {#text} at (4,4) size 163x18
    8383                  text run at (4,4) width 163: "TABLE Testing Section"
    84           RenderTableRow {TR} at (0,26) size 767x526
    85             RenderTableCell {TD} at (0,276) size 12x26 [bgcolor=#C0C0C0] [border: (1px inset #808080)] [r=1 c=0 rs=1 cs=1]
     84          RenderTableRow {TR} at (0,26) size 767x532
     85            RenderTableCell {TD} at (0,279) size 12x26 [bgcolor=#C0C0C0] [border: (1px inset #808080)] [r=1 c=0 rs=1 cs=1]
    8686              RenderText {#text} at (4,4) size 4x18
    8787                text run at (4,4) width 4: " "
    88             RenderTableCell {TD} at (12,26) size 755x526 [border: (1px inset #808080)] [r=1 c=1 rs=1 cs=1]
     88            RenderTableCell {TD} at (12,26) size 755x532 [border: (1px inset #808080)] [r=1 c=1 rs=1 cs=1]
    8989              RenderBlock {DIV} at (4,4) size 747x199
    9090                RenderBlock {P} at (0,0) size 747x18
    … …  
    110110                RenderText {#text} at (0,0) size 396x18
    111111                  text run at (0,0) width 396: "This sentence should be in a sans-serif font, not cursive."
    112               RenderBlock {P} at (4,268) size 747x15
    113                 RenderText {#text} at (0,0) size 440x15
    114                   text run at (0,0) width 440: "This sentence should be in a monospace font, not serif."
    115               RenderBlock {HR} at (4,296) size 747x2 [border: (1px inset #000000)]
    116               RenderBlock {DIV} at (4,311) size 747x211
     112              RenderBlock {P} at (4,268) size 747x18
     113                RenderText {#text} at (0,0) size 550x18
     114                  text run at (0,0) width 550: "This sentence should be in a monospace font, not serif."
     115              RenderBlock {HR} at (4,302) size 747x2 [border: (1px inset #000000)]
     116              RenderBlock {DIV} at (4,317) size 747x211
    117117                RenderBlock {P} at (0,0) size 747x30
    118118                  RenderText {#text} at (0,0) size 728x30

  • trunk/LayoutTests/platform/mac/css2.1/t1503-c522-font-family-00-b-expected.txt

    r25970 r47229  
    11layer at (0,0) size 800x600
    22  RenderView at (0,0) size 800x600
    3 layer at (0,0) size 800x284
    4   RenderBlock {HTML} at (0,0) size 800x284
    5     RenderBody {BODY} at (8,8) size 784x268
     3layer at (0,0) size 800x287
     4  RenderBlock {HTML} at (0,0) size 800x287
     5    RenderBody {BODY} at (8,8) size 784x271
    66      RenderBlock {DIV} at (0,0) size 784x119
    77        RenderBlock {P} at (0,0) size 784x18 [color=#000080]
    … …  
    2626        RenderText {#text} at (0,0) size 311x18
    2727          text run at (0,0) width 311: "This sentence should be in a sans-serif font."
    28       RenderBlock {P} at (0,137) size 784x15 [color=#000080]
    29         RenderText {#text} at (0,0) size 352x15
    30           text run at (0,0) width 352: "This sentence should be in a monospace font."
    31       RenderBlock {DIV} at (0,152) size 784x116
     28      RenderBlock {P} at (0,137) size 784x18 [color=#000080]
     29        RenderText {#text} at (0,0) size 440x18
     30          text run at (0,0) width 440: "This sentence should be in a monospace font."
     31      RenderBlock {DIV} at (0,155) size 784x116
    3232        RenderBlock {P} at (0,0) size 784x15 [color=#000080]
    3333          RenderText {#text} at (0,0) size 352x15

  • trunk/LayoutTests/platform/mac/fast/text/monospace-width-cache-expected.txt

    r29900 r47229  
    2121        RenderText {#text} at (0,0) size 369x18
    2222          text run at (0,0) width 369: "The blue line and the black line should be the same length."
    23       RenderBlock {P} at (0,86) size 784x15
    24         RenderInline {SPAN} at (0,0) size 156x16 [border: (1px solid #0000FF) none]
    25           RenderText {#text} at (0,0) size 156x15
    26             text run at (0,0) width 156: "\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}"
     23      RenderBlock {P} at (0,86) size 784x18
     24        RenderInline {SPAN} at (0,0) size 192x19 [border: (1px solid #0000FF) none]
     25          RenderText {#text} at (0,0) size 192x18
     26            text run at (0,0) width 192: "\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}\x{2500}"

  • trunk/WebCore/ChangeLog

    r47227 r47229  
     12009-08-13  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by David Hyatt.
     4
     5        wrong font size when css font-family includes monospace
     6        https://bugs.webkit.org/show_bug.cgi?id=19161
     7
     8        Firefox only uses fixed-width default size for exactly "font-family: monospace;".
     9        WebKit has historically used fixed-width default size any time a
     10        font-family includes monospace in the fallback list.
     11
     12        This patch corrects WebKit's behavior to match Firefox.
     13        I also had to fix a bug in WebKit's font-family fallback behavior where
     14        child elements would inherit parts of their parents fallback lists.
     15
     16        This patch is mostly just replacing all cases where we used to check for:
     17        fontDescription.genericFontFamily() == MonospaceFamily
     18        with:
     19        fontDescription.useFixedDefaultSize()
     20
     21        Tests: fast/css/getComputedStyle/computed-style-font-family-monospace.html
     22               fast/css/getComputedStyle/font-family-fallback-reset.html
     23
     24        * css/CSSStyleSelector.cpp:
     25        (WebCore::CSSStyleSelector::applyProperty):
     26           Deploy useFixedDefaultSize().  Also fix the bug where child
     27           FontDescriptions would carry part of the parent font-family fallback list.
     28        (WebCore::CSSStyleSelector::checkForGenericFamilyChange):
     29           It's no longer alright to just check genericFontFamily(),
     30           we have to check to make sure the changed style has a matching useFixedDefaultSize().
     31        * platform/graphics/FontDescription.h:
     32        (WebCore::FontDescription::useFixedDefaultSize):
     33           Only use the fixed default size if we have one font family and it is "monospace".
     34           "-webkit-monospace" is the internal representation of the CSS identifier "monospace".
     35
    1362009-08-13  Christian Plesner Hansen  <christian.plesner.hansen@gmail.com>
    237

  • trunk/WebCore/css/CSSStyleSelector.cpp

    r47176 r47229  
    37803780        FontDescription fontDescription = m_style->fontDescription();
    37813781        fontDescription.setKeywordSize(0);
    3782         bool familyIsFixed = fontDescription.genericFamily() == FontDescription::MonospaceFamily;
    37833782        float oldSize = 0;
    37843783        float size = 0;
    … …  
    37953794                fontDescription.setKeywordSize(m_parentStyle->fontDescription().keywordSize());
    37963795        } else if (isInitial) {
    3797             size = fontSizeForKeyword(CSSValueMedium, m_style->htmlHacks(), familyIsFixed);
     3796            size = fontSizeForKeyword(CSSValueMedium, m_style->htmlHacks(), fontDescription.useFixedDefaultSize());
    37983797            fontDescription.setKeywordSize(CSSValueMedium - CSSValueXxSmall + 1);
    37993798        } else if (primitiveValue->getIdent()) {
    … …  
    38083807                case CSSValueXxLarge:
    38093808                case CSSValueWebkitXxxLarge:
    3810                     size = fontSizeForKeyword(primitiveValue->getIdent(), m_style->htmlHacks(), familyIsFixed);
     3809                    size = fontSizeForKeyword(primitiveValue->getIdent(), m_style->htmlHacks(), fontDescription.useFixedDefaultSize());
    38113810                    fontDescription.setKeywordSize(primitiveValue->getIdent() - CSSValueXxSmall + 1);
    38123811                    break;
    … …  
    40514050                m_fontDirty = true;
    40524051            return;
    4053         }
    4054         else if (isInitial) {
     4052        } else if (isInitial) {
    40554053            FontDescription initialDesc = FontDescription();
    40564054            FontDescription fontDescription = m_style->fontDescription();
    40574055            // We need to adjust the size to account for the generic family change from monospace
    40584056            // to non-monospace.
    4059             if (fontDescription.keywordSize() && fontDescription.genericFamily() == FontDescription::MonospaceFamily)
     4057            if (fontDescription.keywordSize() && fontDescription.useFixedDefaultSize())
    40604058                setFontSize(fontDescription, fontSizeForKeyword(CSSValueXxSmall + fontDescription.keywordSize() - 1, m_style->htmlHacks(), false));
    40614059            fontDescription.setGenericFamily(initialDesc.genericFamily());
    … …  
    40674065        }
    40684066       
    4069         if (!value->isValueList()) return;
     4067        if (!value->isValueList())
     4068            return;
    40704069        FontDescription fontDescription = m_style->fontDescription();
    4071         CSSValueList *list = static_cast<CSSValueList*>(value);
     4070        CSSValueList* list = static_cast<CSSValueList*>(value);
    40724071        int len = list->length();
    40734072        FontFamily& firstFamily = fontDescription.firstFamily();
    4074         FontFamily *currFamily = 0;
     4073        FontFamily* currFamily = 0;
    40754074       
    40764075        // Before mapping in a new font-family property, we should reset the generic family.
    4077         bool oldFamilyIsMonospace = fontDescription.genericFamily() == FontDescription::MonospaceFamily;
     4076        bool oldFamilyUsedFixedDefaultSize = fontDescription.useFixedDefaultSize();
    40784077        fontDescription.setGenericFamily(FontDescription::NoFamily);
    40794078
    40804079        for (int i = 0; i < len; i++) {
    4081             CSSValue *item = list->itemWithoutBoundsCheck(i);
    4082             if (!item->isPrimitiveValue()) continue;
    4083             CSSPrimitiveValue *val = static_cast<CSSPrimitiveValue*>(item);
     4080            CSSValue* item = list->itemWithoutBoundsCheck(i);
     4081            if (!item->isPrimitiveValue())
     4082                continue;
     4083            CSSPrimitiveValue* val = static_cast<CSSPrimitiveValue*>(item);
    40844084            AtomicString face;
    40854085            Settings* settings = m_checker.m_document->settings();
    … …  
    41134113                }
    41144114            }
    4115    
     4115
    41164116            if (!face.isEmpty()) {
    41174117                if (!currFamily) {
    41184118                    // Filling in the first family.
    41194119                    firstFamily.setFamily(face);
     4120                    firstFamily.appendFamily(0); // Remove any inherited family-fallback list.
    41204121                    currFamily = &firstFamily;
    4121                 }
    4122                 else {
     4122                } else {
    41234123                    RefPtr<SharedFontFamily> newFamily = SharedFontFamily::create();
    41244124                    newFamily->setFamily(face);
    … …  
    41264126                    currFamily = newFamily.get();
    41274127                }
    4128    
    4129                 if (fontDescription.keywordSize() && (fontDescription.genericFamily() == FontDescription::MonospaceFamily) != oldFamilyIsMonospace)
    4130                     setFontSize(fontDescription, fontSizeForKeyword(CSSValueXxSmall + fontDescription.keywordSize() - 1, m_style->htmlHacks(), !oldFamilyIsMonospace));
    4131            
    4132                 if (m_style->setFontDescription(fontDescription))
    4133                     m_fontDirty = true;
    4134             }
    4135         }
    4136       return;
     4128            }
     4129        }
     4130
     4131        // We can't call useFixedDefaultSize() until all new font families have been added
     4132        // If currFamily is non-zero then we set at least one family on this description.
     4133        if (currFamily) {
     4134            if (fontDescription.keywordSize() && fontDescription.useFixedDefaultSize() != oldFamilyUsedFixedDefaultSize)
     4135                setFontSize(fontDescription, fontSizeForKeyword(CSSValueXxSmall + fontDescription.keywordSize() - 1, m_style->htmlHacks(), !oldFamilyUsedFixedDefaultSize));
     4136
     4137            if (m_style->setFontDescription(fontDescription))
     4138                m_fontDirty = true;
     4139        }
     4140        return;
    41374141    }
    41384142    case CSSPropertyTextDecoration: {
    … …  
    56295633
    56305634    const FontDescription& parentFont = parentStyle->fontDescription();
    5631 
    5632     if (childFont.genericFamily() == parentFont.genericFamily())
     5635    if (childFont.useFixedDefaultSize() == parentFont.useFixedDefaultSize())
    56335636        return;
    56345637
    … …  
    56435646    // multiplying by our scale factor.
    56445647    float size;
    5645     if (childFont.keywordSize()) {
    5646         size = fontSizeForKeyword(CSSValueXxSmall + childFont.keywordSize() - 1, style->htmlHacks(),
    5647                                   childFont.genericFamily() == FontDescription::MonospaceFamily);
    5648     } else {
     5648    if (childFont.keywordSize())
     5649        size = fontSizeForKeyword(CSSValueXxSmall + childFont.keywordSize() - 1, style->htmlHacks(), childFont.useFixedDefaultSize());
     5650    else {
    56495651        Settings* settings = m_checker.m_document->settings();
    56505652        float fixedScaleFactor = settings
    56515653            ? static_cast<float>(settings->defaultFixedFontSize()) / settings->defaultFontSize()
    56525654            : 1;
    5653         size = (parentFont.genericFamily() == FontDescription::MonospaceFamily) ?
    5654                 childFont.specifiedSize()/fixedScaleFactor :
    5655                 childFont.specifiedSize()*fixedScaleFactor;
     5655        size = parentFont.useFixedDefaultSize() ?
     5656                childFont.specifiedSize() / fixedScaleFactor :
     5657                childFont.specifiedSize() * fixedScaleFactor;
    56565658    }
    56575659

  • trunk/WebCore/platform/graphics/FontDescription.h

    r43243 r47229  
    8181    GenericFamilyType genericFamily() const { return static_cast<GenericFamilyType>(m_genericFamily); }
    8282    bool usePrinterFont() const { return m_usePrinterFont; }
     83    // only use fixed default size when there is only one font family, and that family is "monospace"
     84    bool useFixedDefaultSize() const { return genericFamily() == MonospaceFamily && !family().next() && family().family() == "-webkit-monospace"; }
    8385    FontRenderingMode renderingMode() const { return static_cast<FontRenderingMode>(m_renderingMode); }
    8486    unsigned keywordSize() const { return m_keywordSize; }
Note: See TracChangeset for help on using the changeset viewer.