Changeset 183366 in webkit


Ignore:
Timestamp:
Apr 26, 2015 2:25:02 PM (9 years ago)
Author:
Darin Adler
Message:

REGRESSION (r176751): line-height ignored in <button> elements
https://bugs.webkit.org/show_bug.cgi?id=144234

Reviewed by Antti Koivisto.

Source/WebCore:

Test: fast/forms/button-line-height.html

  • platform/Theme.h: Changed controlFont to return an Optional so we can tell

when the theme is overriding the font. Otherwise if the font from the user-agent
style sheet and the font from the theme are the same, we will think we are not
overriding the font when we actually are.

  • platform/mac/ThemeMac.h: Updated controlFont to return Optional.
  • platform/mac/ThemeMac.mm:

(WebCore::ThemeMac::controlFont): Ditto.

  • rendering/RenderTheme.cpp:

(WebCore::RenderTheme::adjustStyle): Set line height only if the font is
overriden by the theme, all the time for PushButtonPart on Mac, and not at all
for other parts. Also tightened up the logic a little since RenderStyle's
setFontDescription already does an "==" comparison; we don't have to do
that twice.

LayoutTests:

  • fast/forms/button-line-height-expected.html: Added.
  • fast/forms/button-line-height.html: Added.
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r183365 r183366  
     12015-04-26  Darin Adler  <darin@apple.com>
     2
     3        REGRESSION (r176751): line-height ignored in <button> elements
     4        https://bugs.webkit.org/show_bug.cgi?id=144234
     5
     6        Reviewed by Antti Koivisto.
     7
     8        * fast/forms/button-line-height-expected.html: Added.
     9        * fast/forms/button-line-height.html: Added.
     10
    1112015-04-26  Darin Adler  <darin@apple.com>
    212
  • trunk/Source/WebCore/ChangeLog

    r183365 r183366  
     12015-04-26  Darin Adler  <darin@apple.com>
     2
     3        REGRESSION (r176751): line-height ignored in <button> elements
     4        https://bugs.webkit.org/show_bug.cgi?id=144234
     5
     6        Reviewed by Antti Koivisto.
     7
     8        Test: fast/forms/button-line-height.html
     9
     10        * platform/Theme.h: Changed controlFont to return an Optional so we can tell
     11        when the theme is overriding the font. Otherwise if the font from the user-agent
     12        style sheet and the font from the theme are the same, we will think we are not
     13        overriding the font when we actually are.
     14
     15        * platform/mac/ThemeMac.h: Updated controlFont to return Optional.
     16        * platform/mac/ThemeMac.mm:
     17        (WebCore::ThemeMac::controlFont): Ditto.
     18
     19        * rendering/RenderTheme.cpp:
     20        (WebCore::RenderTheme::adjustStyle): Set line height only if the font is
     21        overriden by the theme, all the time for PushButtonPart on Mac, and not at all
     22        for other parts. Also tightened up the logic a little since RenderStyle's
     23        setFontDescription already does an "==" comparison; we don't have to do
     24        that twice.
     25
    1262015-04-26  Darin Adler  <darin@apple.com>
    227
  • trunk/Source/WebCore/platform/Theme.h

    r178510 r183366  
    3535#include "ThemeTypes.h"
    3636#include <wtf/Forward.h>
     37#include <wtf/Optional.h>
    3738
    3839namespace WebCore {
     
    8384   
    8485    // The font description result should have a zoomed font size.
    85     virtual FontDescription controlFont(ControlPart, const FontCascade& font, float /*zoomFactor*/) const { return font.fontDescription(); }
     86    virtual Optional<FontDescription> controlFont(ControlPart, const FontCascade&, float /*zoomFactor*/) const { return Nullopt; }
    8687   
    87     // The size here is in zoomed coordinates already.  If a new size is returned, it also needs to be in zoomed coordinates.
     88    // The size here is in zoomed coordinates already. If a new size is returned, it also needs to be in zoomed coordinates.
    8889    virtual LengthSize controlSize(ControlPart, const FontCascade&, const LengthSize& zoomedSize, float /*zoomFactor*/) const { return zoomedSize; }
    8990   
  • trunk/Source/WebCore/platform/mac/ThemeMac.h

    r182068 r183366  
    4343    virtual int baselinePositionAdjustment(ControlPart) const;
    4444
    45     virtual FontDescription controlFont(ControlPart, const FontCascade&, float zoomFactor) const;
     45    virtual Optional<FontDescription> controlFont(ControlPart, const FontCascade&, float zoomFactor) const;
    4646   
    4747    virtual LengthSize controlSize(ControlPart, const FontCascade&, const LengthSize&, float zoomFactor) const;
  • trunk/Source/WebCore/platform/mac/ThemeMac.mm

    r179450 r183366  
    668668}
    669669
    670 FontDescription ThemeMac::controlFont(ControlPart part, const FontCascade& font, float zoomFactor) const
     670Optional<FontDescription> ThemeMac::controlFont(ControlPart part, const FontCascade& font, float zoomFactor) const
    671671{
    672672    switch (part) {
  • trunk/Source/WebCore/rendering/RenderTheme.cpp

    r180720 r183366  
    171171               
    172172        // Font
    173         FontDescription controlFont = m_theme->controlFont(part, style.fontCascade(), style.effectiveZoom());
    174         if (controlFont != style.fontCascade().fontDescription()) {
    175             // Now update our font.
    176             if (style.setFontDescription(controlFont))
    177                 style.fontCascade().update(0);
     173        if (auto themeFont = m_theme->controlFont(part, style.fontCascade(), style.effectiveZoom())) {
     174            // If overriding the specified font with the theme font, also override the line height with the standard line height.
     175            style.setLineHeight(RenderStyle::initialLineHeight());
     176            if (style.setFontDescription(themeFont.value()))
     177                style.fontCascade().update(nullptr);
    178178        }
    179         // Reset our line-height
    180         style.setLineHeight(RenderStyle::initialLineHeight());
    181179        style.setInsideDefaultButton(part == DefaultButtonPart);
    182180    }
Note: See TracChangeset for help on using the changeset viewer.