[webkit-reviews] review granted: [Bug 70304] width/height attributes of input element should be supported when the type of the input element is image. : [Attachment 117590] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Dec 2 09:09:59 PST 2011


Darin Adler <darin at apple.com> has granted Dongwoo Joshua Im
<dw.im at samsung.com>'s request for review:
Bug 70304: width/height attributes of input element should be supported when
the type of the input element is image.
https://bugs.webkit.org/show_bug.cgi?id=70304

Attachment 117590: Patch
https://bugs.webkit.org/attachment.cgi?id=117590&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
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.


More information about the webkit-reviews mailing list