[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


Kent Tamura <tkent at chromium.org> changed:

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




--- Comment #25 from Kent Tamura <tkent at chromium.org>  2011-11-03 03:53:05 PST ---
(From update of attachment 113447)
View in context: https://bugs.webkit.org/attachment.cgi?id=113447&action=review

> LayoutTests/ChangeLog:14
> +        * fast/forms/resources/samsung.gif: Added.

Are you sure that this image can be distributed in WebKit's license?

> LayoutTests/fast/forms/input-width-height-attributes.html:4
> +    <link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">

This line is not needed.

> LayoutTests/fast/forms/input-width-height-attributes.html:42
> +    <div id="console"></div>

This line is not needed. js-test-pre.js automatically creates it.

> LayoutTests/fast/forms/input-width-height-attributes.html:45
> +    <script>
> +        description('width and height attributes of HTMLInputElement.<br>');

nit: Indenting all of the content is meaningless.

> LayoutTests/fast/forms/input-width-height-attributes.html:50
> +        div1.innerHTML = "<b>case #1.</b> Image type. --- HTML inline setting as \"160\", \"80px\","
> +                          + " result is " + image1.width + ", " + image1.height + "<br>" + div1.innerHTML;

We had better show "PASS" or "FAIL" in order to tell what is expected to other developers.

> Source/WebCore/html/HTMLInputElement.cpp:1017
> +int HTMLInputElement::height() const

Why is this "int"?

> Source/WebCore/html/HTMLInputElement.cpp:1022
> +    if (m_inputType->shouldRespectHeightAndWidthAttributes())

We prefer early exit. So, this function should be:

{
    if (!m_inputType->shouldRespectHeightAndWidthAttributes())
        return 0;
    :
    :

> Source/WebCore/html/HTMLInputElement.cpp:1023
> +        height = fastGetAttribute(heightAttr).toInt(&ok);

We had better use parseHTMLNonNegativeInteger().

Also, this implementation is incorrect.  The specification says height/width returns the intrinsic size of the image.  Please refer to the implementation of HTMLImageElement::width() and height().

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