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):

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

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index b5884d7e1716a519cdfafcb7fcd56fd70d5ab26f..323a6f71ec82a2a6dfd02df262e3a0a2d198568c 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -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

diff --git a/WebCore/css/cssstyleselector.cpp b/WebCore/css/cssstyleselector.cpp
index 966809f509cabdb3281e94d331b6951e24dda31f..9c2d7efd9494523d90a4c6434865db26ebdc68a9 100644 (file)
--- a/WebCore/css/cssstyleselector.cpp
+++ b/WebCore/css/cssstyleselector.cpp
@@ -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

diff --git a/WebCore/css/html4.css b/WebCore/css/html4.css
index 5280e2b42d316601b1102f6932009dd239916a29..39f3dae7766edf79aa30ed41f883be3657fb9317 100644 (file)
--- a/WebCore/css/html4.css
+++ b/WebCore/css/html4.css
@@ -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;

diff --git a/WebCore/rendering/DeprecatedRenderSelect.h b/WebCore/rendering/DeprecatedRenderSelect.h
index ffc1d6b86ddba8e3aeb8815fd53566e729b9021b..29a1d79566440b60d70e20dc8244ee0093150b57 100644 (file)
--- a/WebCore/rendering/DeprecatedRenderSelect.h
+++ b/WebCore/rendering/DeprecatedRenderSelect.h
@@ -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();

diff --git a/WebCore/rendering/DeprecatedSlider.h b/WebCore/rendering/DeprecatedSlider.h
index f803906a104c8ec6a76dd8f0b2146a624cb46946..72142e920eb78116c167215a0015c851e1318993 100644 (file)
--- a/WebCore/rendering/DeprecatedSlider.h
+++ b/WebCore/rendering/DeprecatedSlider.h
@@ -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();
 

diff --git a/WebCore/rendering/RenderFormElement.cpp b/WebCore/rendering/RenderFormElement.cpp
index 29e9957952b28126e93daceac71a044fe1bd3ddc..47efe7f74ddc47957de503816bba02fd3d320383 100644 (file)
--- a/WebCore/rendering/RenderFormElement.cpp
+++ b/WebCore/rendering/RenderFormElement.cpp
@@ -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

diff --git a/WebCore/rendering/RenderFormElement.h b/WebCore/rendering/RenderFormElement.h
index 0a546be5ca8589ce2a24fd53d73a44fbbc6629f2..281d275851f25b6ca0b745741ec2f8b4311337b5 100644 (file)
--- a/WebCore/rendering/RenderFormElement.h
+++ b/WebCore/rendering/RenderFormElement.h
@@ -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();
 

diff --git a/WebCore/rendering/RenderLayer.cpp b/WebCore/rendering/RenderLayer.cpp
index 83066a893629beba56b654a4b6bb3831a1281281..12444570c7dcbe857617f222001d65220444c6e5 100644 (file)
--- a/WebCore/rendering/RenderLayer.cpp
+++ b/WebCore/rendering/RenderLayer.cpp
@@ -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);
         }

diff --git a/WebCore/rendering/RenderThemeMac.h b/WebCore/rendering/RenderThemeMac.h
index 4f83039b929377a83c53c4607550d899e55679d4..1738199d39ce0703f0e3a7cb31c9c1191ac1537e 100644 (file)
--- a/WebCore/rendering/RenderThemeMac.h
+++ b/WebCore/rendering/RenderThemeMac.h
@@ -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*);

diff --git a/WebCore/rendering/RenderThemeMac.mm b/WebCore/rendering/RenderThemeMac.mm
index 7b6d38b1c8c9cd2339363a40649cf267ca24f70b..8e4c6b0eb10fc50908a49b3d4eb50a664acaece3 100644 (file)
--- a/WebCore/rendering/RenderThemeMac.mm
+++ b/WebCore/rendering/RenderThemeMac.mm
@@ -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;
     

diff --git a/WebCore/rendering/RenderThemeWin.cpp b/WebCore/rendering/RenderThemeWin.cpp
index 30bb39de24acaee00f0fae8c8f486a7ccba9883c..6a26a33006bc241dfaf359a034328b2d1b7f3054 100644 (file)
--- a/WebCore/rendering/RenderThemeWin.cpp
+++ b/WebCore/rendering/RenderThemeWin.cpp
@@ -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)