[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 #139966|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #62 from Kent Tamura <tkent at chromium.org>  2012-05-04 19:35:31 PST ---
(From update of attachment 139966)
View in context: https://bugs.webkit.org/attachment.cgi?id=139966&action=review

> LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image-expected.txt:5
> +TEST COMPLETE
> +PASS ('width' in e) is true

'TEST COMPLETE,' then 'PASS' looks confusing.  Please follow my comments below.

> LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image.html:9
> +if (window.layoutTestController) {
> +    layoutTestController.waitUntilDone();
> +    layoutTestController.dumpAsText();
> +}

Replace these lines with:
  jsTestIsAsync = true;

> LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image.html:16
> +function log(text)
> +{
> +    var console = document.getElementById('console');
> +    console.appendChild(document.createTextNode(text));
> +    console.appendChild(document.createElement('br'));
> +}

Remove this function.  js-test-pre.js provides debug() function.

> LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image.html:20
> +<input type="image" id="imageSlow" src="resources/image-slow.pl">

This element has a renderer.  You need to specify style="display:none;"

> LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image.html:28
> +            shouldBeTrue("('width' in e)");
> +            shouldBeTrue("('height' in e)");

They are meaningless because HTMLInputElement always has width/height properties.
You should verify what values should width/height be.

> LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image.html:29
> +            log("Success")

This message is meaningless. We can remove it.

> LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image.html:30
> +            layoutTestController.notifyDone()

Replace this with:
  finishJSTest();

> LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image.html:32
> +        } else if (readyState == 'loading' || readyState == 'interactive') {
> +            setTimeout("runTest()", 50);

Polling readyState is not efficient.  You should use events such as 'load' 'readystatechange'.

> LayoutTests/fast/forms/input-width-height-attributes-without-renderer-not-loaded-image.html:20
> +<input type="image" id="imageSlow" src="resources/image-slow.pl">

This has a renderer.

> LayoutTests/fast/forms/input-width-height-attributes-without-renderer-not-loaded-image.html:37
> +    function runTest() {
> +        readyState = document.readyState;
> +        if (readyState == 'complete') {
> +            log("Success")
> +            layoutTestController.notifyDone()
> +        } else if (readyState == 'loading' || readyState == 'interactive') {
> +            shouldBeTrue("('width' in e)");
> +            shouldBeTrue("('height' in e)");
> +
> +            setTimeout("runTest()", 50);
> +        }
> +    }
> +
> +    runTest();

I don't think this test needs to be asynchronous.

var e = document.getELementById("ImageSlow");
shouldBe(...)

is enough.

> LayoutTests/fast/forms/input-width-height-attributes-without-renderer.html:15
> +<input id="inputElement" type="image" width="50" height="50">

This element has a renderer.

> LayoutTests/fast/forms/input-width-height-attributes-without-renderer.html:16
> +No crash = test PASS

Why? 
We should verify what width/height should be.

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