[Webkit-unassigned] [Bug 60673] <input type=number> doesn't ignore size="" attribute

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=60673


Kent Tamura <tkent at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #102525|review?                     |review-
               Flag|                            |




--- Comment #10 from Kent Tamura <tkent at chromium.org>  2011-08-01 22:15:57 PST ---
(From update of attachment 102525)
View in context: https://bugs.webkit.org/attachment.cgi?id=102525&action=review

> LayoutTests/fast/forms/input-number-size.html:60
> +number.size = 10;
> +shouldBe('number.offsetWidth', 'text.offsetWidth');

These lines should be:
  shouldBe('number.size = 10; number.offsetWidth', 'text.offsetWidth');
to show what is tested.

> LayoutTests/fast/forms/input-number-size.html:63
> +number.size = 100;
> +shouldBe('number.offsetWidth', 'text.offsetWidth');

ditto.

> LayoutTests/fast/forms/input-number-size.html:81
> +shouldBe('numberWidth(0, 10, 1, "style1")', 'textWidth(2) + 20');

Why +20?
- You should define variable for the style1 border width.  e.g. var style1Borders = 20;, and use it
- This test case means the text edit area of the number field doesn't have enough width for two letters because of the width of the spin button, right?

> Source/WebCore/ChangeLog:9
> +        The size of input[type=number] is calculated from min/max/step attributes to show the widest possible value.
> +        If min or max attribute is absent, the default size is used.

You had better explain the behavior of setting min/max/step attributes while the field is shown.

> Source/WebCore/html/HTMLInputElement.cpp:1004
> +int HTMLInputElement::preferredSize() const
> +{
> +    if (m_inputType->shouldRespectSizeAttribute())
> +        return size();
> +
> +    return m_inputType->maxNumberOfCharacters(defaultSize);
> +}

It seems we don't need to introduce two new functions for InputType. We can remove shouldRespectSizeAttribute() and maxNumberOfCharacters(), and add InputType::preferredSize().

> Source/WebCore/html/NumberInputType.cpp:149
> +    if (minValueDouble < -limit || limit < minValueDouble)
> +        return defaultSize;

This limit check is already done in parseToDoubleForNumberType*().

> Source/WebCore/html/NumberInputType.cpp:157
> +    if (maxValueDouble < -limit || limit < maxValueDouble)
> +        return defaultSize;

ditto.

> Source/WebCore/html/NumberInputType.cpp:172
> +    if (stepValue.isEmpty()) {
> +        stepValueDouble = 1;
> +        stepValueDecimalPlaces = 0;
> +    } else {
> +        if (!parseToDoubleForNumberTypeWithDecimalPlaces(stepValue, &stepValueDouble, &stepValueDecimalPlaces))
> +            return defaultSize;

if the step attribute value is an invalid string, the effective step value is "1".  Returning defaultSize looks wrong.

> Source/WebCore/rendering/RenderTextControlSingleLine.cpp:430
> +    HTMLElement* spinButton = innerSpinButtonElement();
> +    if (RenderBox* spinRenderer = spinButton ? spinButton->renderBox() : 0) {
> +        result += spinRenderer->borderLeft() + spinRenderer->borderRight() +
> +                  spinRenderer->paddingLeft() + spinRenderer->paddingRight();
> +    }

Why you add only borders and paddings, and not the content of the spin button?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.