[webkit-reviews] review granted: [Bug 84536] Add test function to get placeholder string : [Attachment 138284] Patch 3

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Kent Tamura <tkent at chromium.org>'s
request for review:
Bug 84536: Add test function to get placeholder string
https://bugs.webkit.org/show_bug.cgi?id=84536

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=138284&action=review


> Source/WebCore/testing/Internals.cpp:397
> +    if (!element->hasTagName(inputTag) && !element->hasTagName(textareaTag))

> +	   return String();
> +    HTMLTextFormControlElement* textControl = toTextFormControl(element);

toTextFormControl returns 0 when it's not a text from control element. So you
can just do:
if (!textControl || !textControl->placeholderShouldBeVisible())
    return String();

> LayoutTests/fast/forms/placeholder-stripped-expected.txt:9
> +PASS internals.visiblePlaceholder(input0) is "first line second line"
> +PASS internals.visiblePlaceholder(input1) is ""
> +PASS internals.visiblePlaceholder(textarea0) is "first line second line"
> +PASS internals.visiblePlaceholder(textarea1) is ""

Should we explicitly test a case when we the place holder is invisible?