[webkit-reviews] review denied: [Bug 84674] Input type=range issue with events not being raised when value set in js : [Attachment 150232] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Kevin Ellis <kevers at chromium.org>'s
request for review:
Bug 84674: Input type=range issue with events not being raised when value set
in js
https://bugs.webkit.org/show_bug.cgi?id=84674

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

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=150232&action=review


The C++ change looks ok.  Need some improvement for tests.

> LayoutTests/fast/events/click-range-slider.html:19
> +function log(s)
> +{
> +    document.getElementById('console').appendChild(document.createTextNode(s
+ "\n"));
> +}

Remove this. This is not used.

> LayoutTests/fast/events/click-range-slider.html:28
> +if (window.testRunner) {
> +    testRunner.dumpAsText();
> +}

Remove this.  js-test-pre.js does it.

> LayoutTests/fast/events/click-range-slider.html:55
> +    shouldBeEqualToString("String(clickCount)", "3");

Should be:
    shouldBe("clickCount", "3");

> LayoutTests/fast/events/onchange-range-slider.html:19
> +function log(s)
> +{
> +    document.getElementById('console').appendChild(document.createTextNode(s
+ "\n"));
> +}

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

> LayoutTests/fast/events/onchange-range-slider.html:23
> +    log ('PASS: change event fired.');

Use testPassed() provided by js-test-pre.js.

> LayoutTests/fast/events/onchange-range-slider.html:30
> +if (window.testRunner) {
> +    testRunner.dumpAsText();
> +}

Remove this.

> LayoutTests/fast/events/onchange-range-slider.html:52
> +	   log('Fail: change event not fired.');

Use testFailed() provided by js-test-pre.js.