Bug 71779 - Convert some placeholder-related tests to reftests
Summary: Convert some placeholder-related tests to reftests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords:
Depends on: 60605
Blocks: 67408
  Show dependency treegraph
 
Reported: 2011-11-08 00:52 PST by Kent Tamura
Modified: 2011-11-28 01:38 PST (History)
5 users (show)

See Also:


Attachments
Patch (1.80 MB, patch)
2011-11-08 00:59 PST, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch 2 (1.55 MB, patch)
2011-11-08 02:26 PST, 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 2011-11-08 00:52:33 PST
Convert some placeholder-related tests to reftests
Comment 1 Kent Tamura 2011-11-08 00:59:22 PST
Created attachment 114008 [details]
Patch
Comment 2 Ryosuke Niwa 2011-11-08 01:20:12 PST
I don't think we should do this until the bug 60605 is fixed because only Chromium bots run ref tests until the bug 60605 is fixed.
Comment 3 WebKit Review Bot 2011-11-08 01:59:39 PST
Comment on attachment 114008 [details]
Patch

Attachment 114008 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10374034

New failing tests:
fast/forms/placeholder-position.html
Comment 4 Kent Tamura 2011-11-08 02:26:32 PST
Created attachment 114019 [details]
Patch 2

Drop placeholder-position.html because Chromium Linux and Windows ignore position of a search cancel button.
Comment 5 Ryosuke Niwa 2011-11-08 08:30:28 PST
I'll repeat that we shouldn't land this patch until the bug 60605 is fixed.
Comment 6 Dominic Cooney 2011-11-20 18:45:37 PST
The blocking bug rniwa mentioned is fixed, so someone could review this now?
Comment 7 Ryosuke Niwa 2011-11-20 19:37:26 PST
Comment on attachment 114019 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=114019&action=review

> LayoutTests/fast/forms/input-placeholder-text-indent-expected.html:10
> +<div style="direction:ltr">

Instead of direction, can we use text-align?

> LayoutTests/fast/forms/search-placeholder-value-changed-expected.html:1
> +<input type="search" placeholder="Test Passed">

Interesting to see how we have a separate test for type="text" (placeholder-set-attribute-expected.html) and type="search" (this).
Comment 8 Kent Tamura 2011-11-27 23:17:38 PST
Comment on attachment 114019 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=114019&action=review

Thank you for reviewing.

>> LayoutTests/fast/forms/input-placeholder-text-indent-expected.html:10
>> +<div style="direction:ltr">
> 
> Instead of direction, can we use text-align?

No. Even if text-align is right, text-indent adds indentation at the left side.

>> LayoutTests/fast/forms/search-placeholder-value-changed-expected.html:1
>> +<input type="search" placeholder="Test Passed">
> 
> Interesting to see how we have a separate test for type="text" (placeholder-set-attribute-expected.html) and type="search" (this).

http://trac.webkit.org/changeset/37488, which added search-placeholder-value-changed.html, is not specific to type=search. We may update this test for type=text.
Comment 9 WebKit Review Bot 2011-11-28 01:38:00 PST
Comment on attachment 114019 [details]
Patch 2

Clearing flags on attachment: 114019

Committed r101233: <http://trac.webkit.org/changeset/101233>
Comment 10 WebKit Review Bot 2011-11-28 01:38:10 PST
All reviewed patches have been landed.  Closing bug.