Bug 84536 - Add test function to get placeholder string
Summary: Add test function to get placeholder string
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords:
Depends on:
Blocks: 83872
  Show dependency treegraph
 
Reported: 2012-04-21 10:34 PDT by Kent Tamura
Modified: 2012-04-23 01:00 PDT (History)
5 users (show)

See Also:


Attachments
Cooked on EWS (7.47 KB, patch)
2012-04-21 10:41 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch (16.11 KB, patch)
2012-04-22 18:18 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch 2 (16.11 KB, patch)
2012-04-22 18:26 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch 3 (15.04 KB, patch)
2012-04-22 19:07 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch for landing (14.71 KB, patch)
2012-04-22 22:35 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2012-04-21 10:34:13 PDT
Add test function to get placeholder string
Comment 1 Kent Tamura 2012-04-21 10:41:52 PDT
Created attachment 138246 [details]
Cooked on EWS
Comment 2 Build Bot 2012-04-21 11:05:47 PDT
Comment on attachment 138246 [details]
Cooked on EWS

Attachment 138246 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12469509
Comment 3 Build Bot 2012-04-21 11:08:21 PDT
Comment on attachment 138246 [details]
Cooked on EWS

Attachment 138246 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12476465
Comment 4 Philippe Normand 2012-04-21 11:09:57 PDT
Comment on attachment 138246 [details]
Cooked on EWS

Attachment 138246 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12476468
Comment 5 Kent Tamura 2012-04-22 18:18:02 PDT
Created attachment 138282 [details]
Patch
Comment 6 Philippe Normand 2012-04-22 18:23:03 PDT
Comment on attachment 138282 [details]
Patch

Attachment 138282 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12482698
Comment 7 Kent Tamura 2012-04-22 18:26:41 PDT
Created attachment 138283 [details]
Patch 2

GTK fix?
Comment 8 Build Bot 2012-04-22 18:54:33 PDT
Comment on attachment 138283 [details]
Patch 2

Attachment 138283 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12477683
Comment 9 Kent Tamura 2012-04-22 19:07:16 PDT
Created attachment 138284 [details]
Patch 3

Attempt to fix Windows build
Comment 10 Ryosuke Niwa 2012-04-22 21:51:52 PDT
Comment on attachment 138284 [details]
Patch 3

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?
Comment 11 Kent Tamura 2012-04-22 22:27:31 PDT
(In reply to comment #10)
> toTextFormControl returns 0 when it's not a text from control element. So you can just do:
> if (!textControl || !textControl->placeholderShouldBeVisible())
>     return String();

You're right.  I'll update the code and land it by CQ.

> 
> > 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?

The original test doesn't have intention to test it.
Comment 12 Kent Tamura 2012-04-22 22:35:12 PDT
Created attachment 138292 [details]
Patch for landing
Comment 13 WebKit Review Bot 2012-04-23 01:00:30 PDT
Comment on attachment 138292 [details]
Patch for landing

Clearing flags on attachment: 138292

Committed r114877: <http://trac.webkit.org/changeset/114877>
Comment 14 WebKit Review Bot 2012-04-23 01:00:41 PDT
All reviewed patches have been landed.  Closing bug.