[Webkit-unassigned] [Bug 70304] width/height attributes of input element should be supported when the type of the input element is image.

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #117590|review?, commit-queue?      |review+, commit-queue-
               Flag|                            |




--- Comment #45 from Darin Adler <darin at apple.com>  2011-12-02 09:09:59 PST ---
(From update of attachment 117590)
View in context: https://bugs.webkit.org/attachment.cgi?id=117590&action=review

Patch looks good. I am not sure that the test case covers all the cases that are different. From reading the height function we need:

    1) no renderer but with width attribute
    2) no renderer and no width attribute with a loaded image
    3) no renderer and no width attribute and no loaded image
    4) with renderer

And then for each of those, zoomed and not zoomed.

> Source/WebCore/html/ImageInputType.cpp:189
> +        if (parseHTMLNonNegativeInteger(element->getAttribute(heightAttr), height))

Should be fastGetAttribute here.

> Source/WebCore/html/ImageInputType.cpp:192
> +        // If the image is available, use its hieght.

Typo here. "hieght".

> Source/WebCore/html/ImageInputType.cpp:194
> +        if (m_imageLoader->image())
> +            return m_imageLoader->image()->imageSizeForRenderer(element->renderer(), 1.0f).height();

Normally we use just "1", not "1.0f" in cases like this.

> Source/WebCore/html/ImageInputType.cpp:210
> +        if (parseHTMLNonNegativeInteger(element->getAttribute(widthAttr), width))

Same comment about fastGetAttribute.

> Source/WebCore/html/InputType.h:266
> +    // Sets and Gets width and height of the button if it is image button.

The word "Gets" should not be capitalized.

I’m also not sure this comment is needed or helpful. Comments that say “why” are helpful. Comments that state information that could become out of date are not as good.

The comment refers to the input element as “the button”, but many input elements are not buttons.

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