Fix for bug 10801, form controls that get styled suddenly lose their
authorhyatt <hyatt@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Sep 2006 23:27:16 +0000 (23:27 +0000)
committerhyatt <hyatt@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Sep 2006 23:27:16 +0000 (23:27 +0000)
        intrinsic margins.  Move the intrinsic margin addition code into
        adjustRenderStyle and get it out of the theme code and the old form control
        code.

        Reviewed by aroben

        * css/cssstyleselector.cpp:
        (WebCore::addIntrinsicMargins):
        (WebCore::CSSStyleSelector::adjustRenderStyle):
        * css/html4.css:
        * rendering/DeprecatedRenderSelect.h:
        (WebCore::DeprecatedRenderSelect::calcReplacedHeight):
        * rendering/DeprecatedSlider.h:
        * rendering/RenderFormElement.cpp:
        (WebCore::RenderFormElement::setStyle):
        * rendering/RenderFormElement.h:
        * rendering/RenderLayer.cpp:
        (WebCore::RenderLayer::resize):
        * rendering/RenderThemeMac.h:
        * rendering/RenderThemeMac.mm:
        (WebCore::RenderThemeMac::adjustButtonStyle):
        (WebCore::RenderThemeMac::adjustTextFieldStyle):
        (WebCore::RenderThemeMac::adjustTextAreaStyle):
        (WebCore::RenderThemeMac::adjustMenuListStyle):
        (WebCore::RenderThemeMac::adjustMenuListButtonStyle):
        * rendering/RenderThemeWin.cpp:
        (WebCore::RenderThemeWin::adjustButtonStyle):
        (WebCore::RenderThemeWin::adjustTextFieldStyle):
        (WebCore::RenderThemeWin::adjustTextAreaStyle):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@16289 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/css/cssstyleselector.cpp
WebCore/css/html4.css
WebCore/rendering/DeprecatedRenderSelect.h
WebCore/rendering/DeprecatedSlider.h
WebCore/rendering/RenderFormElement.cpp
WebCore/rendering/RenderFormElement.h
WebCore/rendering/RenderLayer.cpp
WebCore/rendering/RenderThemeMac.h
WebCore/rendering/RenderThemeMac.mm
WebCore/rendering/RenderThemeWin.cpp

index b5884d7e1716a519cdfafcb7fcd56fd70d5ab26f..323a6f71ec82a2a6dfd02df262e3a0a2d198568c 100644 (file)
@@ -1,3 +1,36 @@
+2006-09-10  David Hyatt  <hyatt@apple.com>
+
+        Fix for bug 10801, form controls that get styled suddenly lose their
+        intrinsic margins.  Move the intrinsic margin addition code into
+        adjustRenderStyle and get it out of the theme code and the old form control
+        code.
+
+        Reviewed by aroben
+
+        * css/cssstyleselector.cpp:
+        (WebCore::addIntrinsicMargins):
+        (WebCore::CSSStyleSelector::adjustRenderStyle):
+        * css/html4.css:
+        * rendering/DeprecatedRenderSelect.h:
+        (WebCore::DeprecatedRenderSelect::calcReplacedHeight):
+        * rendering/DeprecatedSlider.h:
+        * rendering/RenderFormElement.cpp:
+        (WebCore::RenderFormElement::setStyle):
+        * rendering/RenderFormElement.h:
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::resize):
+        * rendering/RenderThemeMac.h:
+        * rendering/RenderThemeMac.mm:
+        (WebCore::RenderThemeMac::adjustButtonStyle):
+        (WebCore::RenderThemeMac::adjustTextFieldStyle):
+        (WebCore::RenderThemeMac::adjustTextAreaStyle):
+        (WebCore::RenderThemeMac::adjustMenuListStyle):
+        (WebCore::RenderThemeMac::adjustMenuListButtonStyle):
+        * rendering/RenderThemeWin.cpp:
+        (WebCore::RenderThemeWin::adjustButtonStyle):
+        (WebCore::RenderThemeWin::adjustTextFieldStyle):
+        (WebCore::RenderThemeWin::adjustTextAreaStyle):
+
 2006-09-10  Darin Adler  <darin@apple.com>
 
         - test for http://bugzilla.opendarwin.org/show_bug.cgi?id=10547
index 966809f509cabdb3281e94d331b6951e24dda31f..9c2d7efd9494523d90a4c6434865db26ebdc68a9 100644 (file)
@@ -966,6 +966,28 @@ RenderStyle* CSSStyleSelector::pseudoStyleForElement(RenderStyle::PseudoId pseud
     return style;
 }
 
+static void addIntrinsicMargins(RenderStyle* style)
+{
+    // Intrinsic margin value.
+    const int intrinsicMargin = 2;
+    
+    // FIXME: Using width/height alone and not also dealing with min-width/max-width is flawed.
+    // FIXME: Using "quirk" to decide the margin wasn't set is kind of lame.
+    if (style->width().isIntrinsicOrAuto()) {
+        if (style->marginLeft().quirk())
+            style->setMarginLeft(Length(intrinsicMargin, Fixed));
+        if (style->marginRight().quirk())
+            style->setMarginRight(Length(intrinsicMargin, Fixed));
+    }
+
+    if (style->height().isAuto()) {
+        if (style->marginTop().quirk())
+            style->setMarginTop(Length(intrinsicMargin, Fixed));
+        if (style->marginBottom().quirk())
+            style->setMarginBottom(Length(intrinsicMargin, Fixed));
+    }
+}
+
 void CSSStyleSelector::adjustRenderStyle(RenderStyle* style, Element *e)
 {
     // Cache our original display.
@@ -1080,10 +1102,19 @@ void CSSStyleSelector::adjustRenderStyle(RenderStyle* style, Element *e)
     // Cull out any useless layers and also repeat patterns into additional layers.
     style->adjustBackgroundLayers();
 
+    // Important: Intrinsic margins get added to controls before the theme has adjusted the style, since the theme will
+    // alter fonts and heights/widths.
+    if (e && e->isControl() && style->fontSize() >= 11) {
+        // Don't apply intrinsic margins to image buttons.  The designer knows how big the images are,
+        // so we have to treat all image buttons as though they were explicitly sized.
+        if (!e->hasTagName(inputTag) || static_cast<HTMLInputElement*>(e)->inputType() != HTMLInputElement::IMAGE)
+            addIntrinsicMargins(style);
+    }
+
     // Let the theme also have a crack at adjusting the style.
     if (style->hasAppearance())
         theme()->adjustStyle(this, style, e, m_hasUAAppearance, m_borderData, m_backgroundData, m_backgroundColor);
-    
+
 #ifdef SVG_SUPPORT
     if (e && e->isSVGElement()) {
         // Spec: http://www.w3.org/TR/SVG/masking.html#OverflowProperty
index 5280e2b42d316601b1102f6932009dd239916a29..39f3dae7766edf79aa30ed41f883be3657fb9317 100644 (file)
@@ -416,7 +416,7 @@ select {
 select[size],
 select[multiple],
 select[size][multiple] {
-    // FIXME: When converting the list box implementation, remove these.
+    /* FIXME: When converting the list box implementation, remove these. */
     -webkit-appearance: none;
     -webkit-box-align: initial;
     box-sizing: initial;
index ffc1d6b86ddba8e3aeb8815fd53566e729b9021b..29a1d79566440b60d70e20dc8244ee0093150b57 100644 (file)
@@ -41,7 +41,6 @@ namespace WebCore {
 
         short baselinePosition(bool f, bool b) const;
         int calcReplacedHeight() const { if (!m_useListBox) return intrinsicHeight(); return RenderFormElement::calcReplacedHeight(); }
-        virtual bool canHaveIntrinsicMargins() const { return true; }
 
         virtual void calcMinMaxWidth();
         virtual void layout();
index f803906a104c8ec6a76dd8f0b2146a624cb46946..72142e920eb78116c167215a0015c851e1318993 100644 (file)
@@ -38,7 +38,6 @@ namespace WebCore {
         
         virtual const char* renderName() const { return "DeprecatedSlider"; }
 
-        virtual bool canHaveIntrinsicMargins() const { return true; }
         virtual void calcMinMaxWidth();
         virtual void updateFromElement();
 
index 29e9957952b28126e93daceac71a044fe1bd3ddc..47efe7f74ddc47957de503816bba02fd3d320383 100644 (file)
@@ -52,9 +52,6 @@ short RenderFormElement::baselinePosition(bool f, bool isRootLineBox) const
 
 void RenderFormElement::setStyle(RenderStyle* s)
 {
-    if (canHaveIntrinsicMargins())
-        addIntrinsicMarginsIfAllowed(s);
-
     RenderWidget::setStyle(s);
 
     // Do not paint a background or border for Aqua form elements
@@ -112,28 +109,4 @@ HorizontalAlignment RenderFormElement::textAlignment() const
     return AlignLeft;
 }
 
-
-void RenderFormElement::addIntrinsicMarginsIfAllowed(RenderStyle* _style)
-{
-    // Cut out the intrinsic margins completely if we end up using mini controls.
-    if (_style->fontSize() < 11)
-        return;
-    
-    // FIXME: Using width/height alone and not also dealing with min-width/max-width is flawed.
-    int m = intrinsicMargin();
-    if (_style->width().isIntrinsicOrAuto()) {
-        if (_style->marginLeft().quirk())
-            _style->setMarginLeft(Length(m, Fixed));
-        if (_style->marginRight().quirk())
-            _style->setMarginRight(Length(m, Fixed));
-    }
-
-    if (_style->height().isAuto()) {
-        if (_style->marginTop().quirk())
-            _style->setMarginTop(Length(m, Fixed));
-        if (_style->marginBottom().quirk())
-            _style->setMarginBottom(Length(m, Fixed));
-    }
-}
-
 } // namespace WebCore
index 0a546be5ca8589ce2a24fd53d73a44fbbc6629f2..281d275851f25b6ca0b745741ec2f8b4311337b5 100644 (file)
@@ -52,16 +52,6 @@ namespace WebCore {
         int paddingLeft() const { return 0; }
         int paddingRight() const { return 0; }
 
-        // Aqua controls use intrinsic margin values in order to leave space for focus rings
-        // and to keep controls from butting up against one another.  This intrinsic margin
-        // is only applied if the Web page allows the control to size intrinsically.  If the
-        // Web page specifies an explicit width for a control, then we go ahead and honor that
-        // precise width.  Similarly, if a Web page uses a specific margin value, we will go ahead
-        // and honor that value.  
-        void addIntrinsicMarginsIfAllowed(RenderStyle*);
-        virtual bool canHaveIntrinsicMargins() const { return false; }
-        int intrinsicMargin() const { return 2; }
-
         virtual void setStyle(RenderStyle*);
         virtual void updateFromElement();
 
index 83066a893629beba56b654a4b6bb3831a1281281..12444570c7dcbe857617f222001d65220444c6e5 100644 (file)
@@ -56,7 +56,6 @@
 #include "OverflowEvent.h"
 #include "PlatformMouseEvent.h"
 #include "RenderArena.h"
-#include "RenderFormElement.h"
 #include "RenderInline.h"
 #include "RenderTheme.h"
 #include "RenderView.h"
@@ -834,7 +833,7 @@ void RenderLayer::resize(const PlatformMouseEvent& evt, const IntSize& offsetFro
     RenderObject* renderer = m_object->node()->shadowAncestorNode()->renderer();
     if (diffWidth && (m_object->style()->resize() == RESIZE_HORIZONTAL || m_object->style()->resize() == RESIZE_BOTH)) {
         CSSStyleDeclaration* style = static_cast<Element*>(m_object->node()->shadowAncestorNode())->style();
-        if (renderer->style()->hasAppearance() || renderer->isFormElement() && static_cast<RenderFormElement*>(renderer)->canHaveIntrinsicMargins()) {
+        if (renderer->element() && renderer->element()->isControl()) {
             style->setProperty(CSS_PROP_MARGIN_LEFT, String::number(renderer->marginLeft()) + "px", false, ec);
             style->setProperty(CSS_PROP_MARGIN_RIGHT, String::number(renderer->marginRight()) + "px", false, ec);
         }
@@ -844,7 +843,7 @@ void RenderLayer::resize(const PlatformMouseEvent& evt, const IntSize& offsetFro
 
     if (diffHeight && (m_object->style()->resize() == RESIZE_VERTICAL || m_object->style()->resize() == RESIZE_BOTH)) {
         CSSStyleDeclaration* style = static_cast<Element*>(m_object->node()->shadowAncestorNode())->style();
-        if (renderer->style()->hasAppearance() || renderer->isFormElement() && static_cast<RenderFormElement*>(renderer)->canHaveIntrinsicMargins()) {
+        if (renderer->element() && renderer->element()->isControl()) {
             style->setProperty(CSS_PROP_MARGIN_TOP, String::number(renderer->marginTop()) + "px", false, ec);
             style->setProperty(CSS_PROP_MARGIN_BOTTOM, String::number(renderer->marginBottom()) + "px", false, ec);
         }
index 4f83039b929377a83c53c4607550d899e55679d4..1738199d39ce0703f0e3a7cb31c9c1191ac1537e 100644 (file)
@@ -96,8 +96,6 @@ private:
     IntSize sizeForFont(RenderStyle*, const IntSize* sizes) const;
     void setFontFromControlSize(CSSStyleSelector*, RenderStyle*, NSControlSize) const;
     
-    void addIntrinsicMargins(RenderStyle*, NSControlSize) const;
-    
     void updateCheckedState(NSCell*, const RenderObject*);
     void updateEnabledState(NSCell*, const RenderObject*);
     void updateFocusedState(NSCell*, const RenderObject*);
index 7b6d38b1c8c9cd2339363a40649cf267ca24f70b..8e4c6b0eb10fc50908a49b3d4eb50a664acaece3 100644 (file)
@@ -360,31 +360,6 @@ void RenderThemeMac::setFontFromControlSize(CSSStyleSelector* selector, RenderSt
         style->font().update();
 }
 
-void RenderThemeMac::addIntrinsicMargins(RenderStyle* style, NSControlSize size) const
-{
-    // Cut out the intrinsic margins completely if we end up using mini controls.
-    if (size == NSMiniControlSize)
-        return;
-    
-    // Intrinsic margin value.
-    const int m = 2;
-    
-    // FIXME: Using width/height alone and not also dealing with min-width/max-width is flawed.
-    if (style->width().isIntrinsicOrAuto()) {
-        if (style->marginLeft().quirk())
-            style->setMarginLeft(Length(m, Fixed));
-        if (style->marginRight().quirk())
-            style->setMarginRight(Length(m, Fixed));
-    }
-
-    if (style->height().isAuto()) {
-        if (style->marginTop().quirk())
-            style->setMarginTop(Length(m, Fixed));
-        if (style->marginBottom().quirk())
-            style->setMarginBottom(Length(m, Fixed));
-    }
-}
-
 bool RenderThemeMac::paintCheckbox(RenderObject* o, const RenderObject::PaintInfo& i, const IntRect& r)
 {
     // Determine the width and height needed for the control and prepare the cell for painting.
@@ -533,9 +508,6 @@ void RenderThemeMac::adjustButtonStyle(CSSStyleSelector* selector, RenderStyle*
     // Determine our control size based off our font.
     NSControlSize controlSize = controlSizeForFont(style);
 
-    // Add in intrinsic margins
-    addIntrinsicMargins(style, controlSize);
-
     if (style->appearance() == PushButtonAppearance) {
         // Ditch the border.
         style->resetBorder();
@@ -657,9 +629,6 @@ bool RenderThemeMac::paintTextField(RenderObject* o, const RenderObject::PaintIn
 
 void RenderThemeMac::adjustTextFieldStyle(CSSStyleSelector* selector, RenderStyle* style, Element* e) const
 {
-    // Add in intrinsic margins if the font size isn't too small
-    if (style->fontSize() >= 11)
-        addIntrinsicMargins(style, NSRegularControlSize);
 }
 
 bool RenderThemeMac::paintTextArea(RenderObject* o, const RenderObject::PaintInfo& paintInfo, const IntRect& r)
@@ -671,9 +640,6 @@ bool RenderThemeMac::paintTextArea(RenderObject* o, const RenderObject::PaintInf
 
 void RenderThemeMac::adjustTextAreaStyle(CSSStyleSelector* selector, RenderStyle* style, Element* e) const
 {
-    // Add in intrinsic margins if the font size isn't too small
-    if (style->fontSize() >= 11)
-        addIntrinsicMargins(style, NSRegularControlSize);
 }
 
 const int* RenderThemeMac::popupButtonMargins() const
@@ -895,9 +861,6 @@ void RenderThemeMac::adjustMenuListStyle(CSSStyleSelector* selector, RenderStyle
 {
     NSControlSize controlSize = controlSizeForFont(style);
 
-    // Add in intrinsic margins
-    addIntrinsicMargins(style, controlSize);
-
     style->resetBorder();
     
     // Height is locked to auto.
@@ -922,11 +885,7 @@ void RenderThemeMac::adjustMenuListStyle(CSSStyleSelector* selector, RenderStyle
 }
 
 void RenderThemeMac::adjustMenuListButtonStyle(CSSStyleSelector* selector, RenderStyle* style, Element* e) const
-{
-    // Add in intrinsic margins if the font size isn't too small
-    if (style->fontSize() >= 11)
-        addIntrinsicMargins(style, NSRegularControlSize);
-        
+{        
     float fontScale = style->fontSize() / baseFontSize;
     float arrowWidth = baseArrowWidth * fontScale;
     
index 30bb39de24acaee00f0fae8c8f486a7ccba9883c..6a26a33006bc241dfaf359a034328b2d1b7f3054 100644 (file)
@@ -147,31 +147,6 @@ Color RenderThemeWin::platformInactiveSelectionForegroundColor() const
     return Color::white;
 }
 
-void RenderThemeWin::addIntrinsicMargins(RenderStyle* style) const
-{
-    // Cut out the intrinsic margins completely if we end up using a small font size
-    if (style->fontSize() < 11)
-        return;
-    
-    // Intrinsic margin value.
-    const int m = 2;
-    
-    // FIXME: Using width/height alone and not also dealing with min-width/max-width is flawed.
-    if (style->width().isIntrinsicOrAuto()) {
-        if (style->marginLeft().quirk())
-            style->setMarginLeft(Length(m, Fixed));
-        if (style->marginRight().quirk())
-            style->setMarginRight(Length(m, Fixed));
-    }
-
-    if (style->height().isAuto()) {
-        if (style->marginTop().quirk())
-            style->setMarginTop(Length(m, Fixed));
-        if (style->marginBottom().quirk())
-            style->setMarginBottom(Length(m, Fixed));
-    }
-}
-
 bool RenderThemeWin::supportsFocus(EAppearance appearance)
 {
     switch (appearance) {
@@ -234,7 +209,6 @@ ThemeData RenderThemeWin::getThemeData(RenderObject* o)
 
 void RenderThemeWin::adjustButtonStyle(CSSStyleSelector*, RenderStyle* style, Element*) const
 {
-    addIntrinsicMargins(style);
 }
 
 static HDC prepareForDrawing(GraphicsContext* g)
@@ -321,7 +295,6 @@ void RenderThemeWin::setRadioSize(RenderStyle* style) const
 
 void RenderThemeWin::adjustTextFieldStyle(CSSStyleSelector*, RenderStyle* style, Element*) const
 {
-    addIntrinsicMargins(style);
 }
 
 bool RenderThemeWin::paintTextField(RenderObject* o, const RenderObject::PaintInfo& i, const IntRect& r)
@@ -351,7 +324,6 @@ bool RenderThemeWin::paintTextField(RenderObject* o, const RenderObject::PaintIn
 
 void RenderThemeWin::adjustTextAreaStyle(CSSStyleSelector*, RenderStyle* style, Element*) const
 {
-    addIntrinsicMargins(style);
 }
 
 bool RenderThemeWin::paintTextArea(RenderObject* o, const RenderObject::PaintInfo& i, const IntRect& r)