Bug 136257

Summary: REGRESSION(r172826) [GTK] [CSS] 4 Layout Tests at fast/forms/*placeholder* and fast/css/placeholder-shown-basics.html fail
Product: WebKit Reporter: Carlos Alberto Lopez Perez <clopez>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cgarcia, dtrebbien, jfernandez, koivisto, rego
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   

Description Carlos Alberto Lopez Perez 2014-08-26 10:14:44 PDT
The following layout tests are failing on platform GTK since r172826 <http://trac.webkit.org/r172826>

Regressions: Unexpected text-only failures (4)

  fast/forms/input-placeholder-visibility-1.html [ Failure ]
    * Diff: http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release%20%28Tests%29/r172826%20%282246%29/fast/forms/input-placeholder-visibility-1-pretty-diff.html
  fast/forms/input-placeholder-visibility-3.html [ Failure ]
    * Diff: http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release%20%28Tests%29/r172826%20%282246%29/fast/forms/input-placeholder-visibility-3-pretty-diff.html
  fast/forms/textarea-placeholder-visibility-1.html [ Failure ]
    * Diff: http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release%20%28Tests%29/r172826%20%282246%29/fast/forms/textarea-placeholder-visibility-1-pretty-diff.html
  fast/forms/textarea-placeholder-visibility-2.html [ Failure ]
    * Diff: http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release%20%28Tests%29/r172826%20%282246%29/fast/forms/textarea-placeholder-visibility-2-pretty-diff.html


As you can see, on the 4 cases there is a renderblock that now is not rendered.
Comment 1 Carlos Alberto Lopez Perez 2014-08-26 11:36:44 PDT
There is another failure related to this bug.

r172826 added a new layout test: fast/css/placeholder-shown-basics.html. This test is failing on platform GTK since it was added.

The diff is:
http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release%20%28Tests%29/r172826%20%282346%29/fast/css/placeholder-shown-basics-diffs.html
Comment 2 Benjamin Poulain 2014-08-26 12:47:00 PDT
Some of it may just need a rebaseline. After r172826, the placeholder no longer gets a RenderObject when not visible.

placeholder-shown-basics.html does not look right.

Can you please debug it? It looks like the <textarea> gets a placeholder but its isPlaceholderVisible() is false.
Comment 3 Benjamin Poulain 2014-08-26 18:25:26 PDT
(In reply to comment #2)
> Some of it may just need a rebaseline. After r172826, the placeholder no longer gets a RenderObject when not visible.
> 
> placeholder-shown-basics.html does not look right.
> 
> Can you please debug it? It looks like the <textarea> gets a placeholder but its isPlaceholderVisible() is false.

Checking this manually. The last field is color, not <textarea>.

It looks like the problem is simply that WebKitGTK does not support the color type. If you want we can remove it from the test, it is not very important.

Can you test the other 4 manually? They may just need a rebase.
Comment 4 Carlos Alberto Lopez Perez 2014-08-27 06:12:54 PDT
I have marked all this tests as failing on the GTK TestExpectations on http://trac.webkit.org/r172981

(In reply to comment #3)
> (In reply to comment #2)
> > Some of it may just need a rebaseline. After r172826, the placeholder no longer gets a RenderObject when not visible.
> > 
> > placeholder-shown-basics.html does not look right.
> > 
> > Can you please debug it? It looks like the <textarea> gets a placeholder but its isPlaceholderVisible() is false.
> 
> Checking this manually. The last field is color, not <textarea>.
> 
> It looks like the problem is simply that WebKitGTK does not support the color type. If you want we can remove it from the test, it is not very important.

Yes, GTK still don't supports that (bug 98935). Given that there are already a bunch of tests for input type color at fast/forms/color maybe removing that from this test can be a good idea so ports that still that don't implement the input color type can pass the test.

> 
> Can you test the other 4 manually? They may just need a rebase.

How can I test them for correctness? any guidelines?

Thanks.
Comment 5 Benjamin Poulain 2014-08-27 18:35:42 PDT
(In reply to comment #4)
> I have marked all this tests as failing on the GTK TestExpectations on http://trac.webkit.org/r172981

Updated the :placeholder-shown part in http://trac.webkit.org/changeset/173042
Comment 6 Benjamin Poulain 2014-08-27 18:44:47 PDT
(In reply to comment #4)
> > Can you test the other 4 manually? They may just need a rebase.
> 
> How can I test them for correctness? any guidelines?
> 
> Thanks.

There is a flag defining if the placeholder should be displayed when an input field is focused (shouldShowPlaceholderWhenFocused). I believe it is false for GTK, and the placeholder should be hidden if the input field is focused. With that assumption:

On every one of those tests, the input field should be focused, and the placeholder should not be displayed. Typing text should not show the placeholder. Emptying the input field and removing the focus should show the placeholder.

From the test results you linked I think it is all good. If you confirm manually, you can just rebaseline those 4 results.


On a completely unrelated: what is the native behavior of GTK for placeholder? Does it also hide on focus? If not we could unify and simplify the code.
Comment 7 Carlos Garcia Campos 2015-10-15 00:54:58 PDT
Yes, this was fixed by r187095, that removed shouldShowPlaceholderWhenFocused in favor of a common behaviour of all ports. Unskipped in r191101