Bug 551846. <select> size should default to 4 when 'multiple' attribute is present. r=smaug, sr=jst

content/base/src/nsAttrValue.cpp

@@ -1099,6 +1099,23 @@ nsAttrValue::ParseNonNegativeIntValue(const nsAString& aString)
   return PR_TRUE;
 }
 
+PRBool
+nsAttrValue::ParsePositiveIntValue(const nsAString& aString)
+{
+  ResetIfSet();
+
+  PRInt32 ec;
+  PRBool strict;
+  PRInt32 originalVal = StringToInteger(aString, &strict, &ec);
+  if (NS_FAILED(ec) || originalVal <= 0) {
+    return PR_FALSE;
+  }
+
+  SetIntValueAndType(originalVal, eInteger, nsnull);
+
+  return PR_TRUE;
+}
+
 void
 nsAttrValue::SetColorValue(nscolor aColor, const nsAString& aString)
 {

content/base/src/nsAttrValue.h

@@ -258,6 +258,21 @@ public:
    */
   PRBool ParseNonNegativeIntValue(const nsAString& aString);
 
+  /**
+   * Parse a string value into a positive integer.
+   * This method follows the rules for parsing non-negative integer from:
+   * http://dev.w3.org/html5/spec/infrastructure.html#rules-for-parsing-non-negative-integers
+   * In addition of these rules, the value has to be greater than zero.
+   *
+   * This is generally used for parsing content attributes which reflecting IDL
+   * attributes are limited to only non-negative numbers greater than zero, see:
+   * http://dev.w3.org/html5/spec/common-dom-interfaces.html#limited-to-only-non-negative-numbers-greater-than-zero
+   *
+   * @param aString       the string to parse
+   * @return              whether the value was valid
+   */
+  PRBool ParsePositiveIntValue(const nsAString& aString);
+
   /**
    * Parse a string into a color.
    *

content/html/content/src/nsGenericHTMLElement.h

@@ -1106,6 +1106,31 @@ NS_NewHTML##_elementName##Element(nsINodeInfo *aNodeInfo, PRBool aFromParser)\
     return SetAttrHelper(nsGkAtoms::_atom, aValue);                       \
   }
 
+/**
+ * A macro to implement the getter and setter for a given content
+ * property that needs to set a positive integer. The method uses
+ * the generic GetAttr and SetAttr methods. This macro is much like
+ * the NS_IMPL_NON_NEGATIVE_INT_ATTR macro except the exception is
+ * thrown also when the value is equal to 0.
+ */
+#define NS_IMPL_POSITIVE_INT_ATTR(_class, _method, _atom)                 \
+  NS_IMPL_POSITIVE_INT_ATTR_DEFAULT_VALUE(_class, _method, _atom, 1)
+
+#define NS_IMPL_POSITIVE_INT_ATTR_DEFAULT_VALUE(_class, _method, _atom, _default)  \
+  NS_IMETHODIMP                                                           \
+  _class::Get##_method(PRInt32* aValue)                                   \
+  {                                                                       \
+    return GetIntAttr(nsGkAtoms::_atom, _default, aValue);                \
+  }                                                                       \
+  NS_IMETHODIMP                                                           \
+  _class::Set##_method(PRInt32 aValue)                                    \
+  {                                                                       \
+    if (aValue <= 0) {                                                    \
+      return NS_ERROR_DOM_INDEX_SIZE_ERR;                                 \
+    }                                                                     \
+    return SetIntAttr(nsGkAtoms::_atom, aValue);                          \
+  }
+
 /**
  * QueryInterface() implementation helper macros
  */

content/html/content/src/nsHTMLSelectElement.cpp

@@ -1206,7 +1206,8 @@ NS_IMPL_BOOL_ATTR(nsHTMLSelectElement, Autofocus, autofocus)
 NS_IMPL_BOOL_ATTR(nsHTMLSelectElement, Disabled, disabled)
 NS_IMPL_BOOL_ATTR(nsHTMLSelectElement, Multiple, multiple)
 NS_IMPL_STRING_ATTR(nsHTMLSelectElement, Name, name)
-NS_IMPL_INT_ATTR_DEFAULT_VALUE(nsHTMLSelectElement, Size, size, 0)
+NS_IMPL_POSITIVE_INT_ATTR_DEFAULT_VALUE(nsHTMLSelectElement, Size, size,
+                                        GetDefaultSize())
 NS_IMPL_INT_ATTR_DEFAULT_VALUE(nsHTMLSelectElement, TabIndex, tabindex, 0)
 
 NS_IMETHODIMP
@@ -1368,7 +1369,7 @@ nsHTMLSelectElement::ParseAttribute(PRInt32 aNamespaceID,
                                     nsAttrValue& aResult)
 {
   if (aAttribute == nsGkAtoms::size && kNameSpaceID_None == aNamespaceID) {
-    return aResult.ParseIntWithBounds(aValue, 0);
+    return aResult.ParsePositiveIntValue(aValue);
   }
   return nsGenericHTMLElement::ParseAttribute(aNamespaceID, aAttribute, aValue,
                                               aResult);

content/html/content/src/nsHTMLSelectElement.h

@@ -473,6 +473,14 @@ protected:
     return PR_TRUE;
   }
 
+  /**
+   * Helper method to get the default size.
+   */
+  PRInt32 GetDefaultSize() const
+  {
+    return HasAttr(kNameSpaceID_None, nsGkAtoms::multiple) ? 4 : 1;
+  }
+
   /** The options[] array */
   nsRefPtr<nsHTMLOptionCollection> mOptions;
   /** false if the parser is in the middle of adding children. */