Bug 71779

Summary: Convert some placeholder-related tests to reftests
Product: WebKit Reporter: Kent Tamura <tkent>
Component: FormsAssignee: Kent Tamura <tkent>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, dominicc, morrita, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 60605    
Bug Blocks: 67408    
Attachments:
Description Flags
Patch
none
Patch 2 none

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.