[webkit-reviews] review denied: [Bug 84351] [Chromium] datalist: Inconsistent behavior of HTMLInputElement::list : [Attachment 138717] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Keishi Hattori
<keishi at webkit.org>'s request for review:
Bug 84351: [Chromium] datalist: Inconsistent behavior of HTMLInputElement::list
https://bugs.webkit.org/show_bug.cgi?id=84351

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

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


> Source/WebCore/rendering/RenderThemeChromiumMac.h:35
> +    bool supportsDataListUI(const AtomicString& type) const OVERRIDE;

Don't you add virtual?

> Source/WebCore/rendering/RenderThemeChromiumSkia.h:55
> +	   bool supportsDataListUI(const AtomicString& type) const OVERRIDE;

ditto.

> LayoutTests/fast/forms/datalist/input-list.html:40
>  <input type="text" id="i4" list="dl1">
> +<input type="search" id="i5" list="dl1">
> +<input type="url" id="i6" list="dl1">
> +<input type="telephone" id="i7" list="dl1">
> +<input type="email" id="i8" list="dl1">
> +<input type="datetime" id="i9" list="dl1">
> +<input type="date" id="i10" list="dl1">
> +<input type="month" id="i11" list="dl1">
> +<input type="week" id="i12" list="dl1">
> +<input type="time" id="i13" list="dl1">
> +<input type="datetime-local" id="i14" list="dl1">
> +<input type="number" id="i15" list="dl1">
> +<input type="range" id="i16" list="dl1">
> +<input type="color" id="i17" list="dl1">
>  <!-- Unsupported type -->
> -<input type="password" id="i5" list="dl1">
> +<input type="hidden" id="i18" list="dl1">
> +<input type="password" id="i19" list="dl1">
> +<input type="checkbox" id="i20" list="dl1">
> +<input type="radio" id="i21" list="dl1">
> +<input type="file" id="i22" list="dl1">
> +<input type="submit" id="i23" list="dl1">
> +<input type="image" id="i24" list="dl1">
> +<input type="reset" id="i25" list="dl1">
> +<input type="button" id="i26" list="dl1">

We had better rename their ID attribute values like
   id="text"
   id="search"
and test them with
    var datalistElement = document.getElementById('dl1');
    shouldBe('document.getElementById("text").list', 'datalistElement');
    shouldBe('document.getElementById("search").list', 'datalistElement');
to improve readability of the test result.

> LayoutTests/platform/chromium/test_expectations.txt:1101
> +// Implementation of datalist is incomplete.
> +BUGWK27247 : fast/forms/datalist/input-list.html = PASS FAIL
> +

This test must not be flay.
We should commit the ideal result as
fast/forms/datalist/input-list-expected.txt, and commit the current Chromium
result as platform/chromium/fast/forms/datalist/input-list-expected.txt. We
shouldn't update test_expectations.txt.