[webkit-reviews] review denied: [Bug 94196] [Tests] Adding tests for multiple fields time input UI : [Attachment 158735] Patch 1

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied yosin at chromium.org's request for
review:
Bug 94196: [Tests] Adding tests for multiple fields time input UI
https://bugs.webkit.org/show_bug.cgi?id=94196

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

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


> LayoutTests/ChangeLog:38
> +

We should have a pixel test for the default appearance of <input type=time
readonly> and <input type=time disabled>

platform/chromium/TestExpectations should have a failing entry for non-Linux.

>
LayoutTests/fast/forms/time-multiple-fields/time-multiple-fields-apperance-basi
c.html:3
> +<link rel="stylesheet" type="text/css"
href="../resources/time-multiple-fields-apperance.css" />

Is this needed?

>
LayoutTests/fast/forms/time-multiple-fields/time-multiple-fields-apperance-pseu
do-elements.html:10
> +    <li><span class="after">After</span><input type="time" value="12:34:56"
class="after"></li>
> +    <li><span class="before">Before</span><input type="time"
value="12:34:56" class="before"></li>

Why do you set after/before to the <span> elements?

>
LayoutTests/fast/forms/time-multiple-fields/time-multiple-fields-apperance-pseu
do-elements.html:11
> +    <li><p id="first-letter">first letter</p><p id="first-letter"><input
type="time" value="12:34:56" class="first-letter"></li></p>

Having multiple element with identical ID is not good.