[svg2] Make 'width' and 'height' presentation attributes

Source/core/svg/SVGRectElement.cpp

 /*
  * Copyright (C) 2004, 2005, 2006, 2008 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007 Rob Buis <buis@kde.org>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
@@ skipping 60 matching lines @@
 }
 
 void SVGRectElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
 {
     parseAttributeNew(name, value);
 }
 
 bool SVGRectElement::isPresentationAttribute(const QualifiedName& attrName) const
 {
     if (attrName == SVGNames::xAttr || attrName == SVGNames::yAttr
+        || attrName == SVGNames::widthAttr || attrName == SVGNames::heightAttr
         || attrName == SVGNames::rxAttr || attrName == SVGNames::ryAttr)
         return true;
     return SVGGeometryElement::isPresentationAttribute(attrName);
 }
 
 bool SVGRectElement::isPresentationAttributeWithSVGDOM(const QualifiedName& attrName) const
 {
     if (attrName == SVGNames::xAttr || attrName == SVGNames::yAttr
+        || attrName == SVGNames::widthAttr || attrName == SVGNames::heightAttr
         || attrName == SVGNames::rxAttr || attrName == SVGNames::ryAttr)
         return true;
     return SVGGeometryElement::isPresentationAttributeWithSVGDOM(attrName);
 }
 
 void SVGRectElement::collectStyleForPresentationAttribute(const QualifiedName& name, const AtomicString& value, MutableStylePropertySet* style)
 {
     RefPtrWillBeRawPtr<SVGAnimatedPropertyBase> property = propertyFromAttribute(name);
     if (property == m_x)
         addSVGLengthPropertyToPresentationAttributeStyle(style, CSSPropertyX, *m_x->currentValue());
     else if (property == m_y)
         addSVGLengthPropertyToPresentationAttributeStyle(style, CSSPropertyY, *m_y->currentValue());
+    else if (property == m_width)
+        addSVGLengthPropertyToPresentationAttributeStyle(style, CSSPropertyWidth, *m_width->currentValue());
+    else if (property == m_height)
+        addSVGLengthPropertyToPresentationAttributeStyle(style, CSSPropertyHeight, *m_height->currentValue());
     else if (property == m_rx)
         addSVGLengthPropertyToPresentationAttributeStyle(style, CSSPropertyRx, *m_rx->currentValue());
     else if (property == m_ry)
         addSVGLengthPropertyToPresentationAttributeStyle(style, CSSPropertyRy, *m_ry->currentValue());
     else
         SVGGeometryElement::collectStyleForPresentationAttribute(name, value, style);
 }
 
 void SVGRectElement::svgAttributeChanged(const QualifiedName& attrName)
 {
     if (!isSupportedAttribute(attrName)) {
         SVGGeometryElement::svgAttributeChanged(attrName);
         return;
     }
 
     SVGElement::InvalidationGuard invalidationGuard(this);
 
-    bool isPresentationLengthAttribute =
-        attrName == SVGNames::xAttr
-        || attrName == SVGNames::yAttr
-        || attrName == SVGNames::rxAttr
-        || attrName == SVGNames::ryAttr;
-    if (isPresentationLengthAttribute) {
-        invalidateSVGPresentationAttributeStyle();
-        setNeedsStyleRecalc(LocalStyleChange,
-            StyleChangeReasonForTracing::fromAttribute(attrName));
-    }
-
-    bool isLengthAttribute = isPresentationLengthAttribute
-        || attrName == SVGNames::widthAttr
-        || attrName == SVGNames::heightAttr;
-
-    if (isLengthAttribute)
-        updateRelativeLengthsInformation();
+    ASSERT(isSupportedAttribute(attrName));

[fs, 2015/03/12 14:28:48]

Probably not the most enlightening assert as long as the method starts the way
it does. Maybe there's something else that could be asserted?
propertyFromAttribute(name)->type() == AnimatedLength?

[Erik Dahlström (inactive), 2015/03/12 16:14:56]

I'm not sure the assert is very useful, it was an attempt at keeping something
similar to the ASSERT_NOT_REACHED.

[fs, 2015/03/12 16:40:07]

I'm fine with dropping the assert altogether. The only case it'd be useful is if
adding a new attribute to <rect> (which seems unlikely enough...)

[Erik Dahlström (inactive), 2015/03/13 09:16:23]

Dropped in patchset #5.

+    invalidateSVGPresentationAttributeStyle();
+    setNeedsStyleRecalc(LocalStyleChange,
+        StyleChangeReasonForTracing::fromAttribute(attrName));
+    updateRelativeLengthsInformation();
 
     LayoutSVGShape* renderer = toLayoutSVGShape(this->layoutObject());
     if (!renderer)
         return;
 
-    if (isLengthAttribute) {
-        renderer->setNeedsShapeUpdate();
-        markForLayoutAndParentResourceInvalidation(renderer);
-        return;
-    }
-
-    ASSERT_NOT_REACHED();
+    renderer->setNeedsShapeUpdate();
+    markForLayoutAndParentResourceInvalidation(renderer);
 }
 
 bool SVGRectElement::selfHasRelativeLengths() const
 {
     return m_x->currentValue()->isRelative()
         || m_y->currentValue()->isRelative()
         || m_width->currentValue()->isRelative()
         || m_height->currentValue()->isRelative()
         || m_rx->currentValue()->isRelative()
         || m_ry->currentValue()->isRelative();
 }
 
 LayoutObject* SVGRectElement::createLayoutObject(const LayoutStyle&)
 {
     return new LayoutSVGRect(this);
 }
 
 } // namespace blink