Bug 119784 – [EFL] fast/forms/datalist/input-list.html is failing

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Ryuan Choi]

fast/forms/datalist/input-list.html is failing on all EFL platforms.

[Santosh Mahto]

Created attachment 208800 [details]
Patch

[Ryuan Choi]

Comment on attachment 208800 [details]
Patch

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

> LayoutTests/platform/efl/fast/forms/datalist/input-list-expected.txt:10
> -FAIL datalist.className should be former. Threw exception TypeError: 'null' is not an object (evaluating 'datalist.className')
> +FAIL datalist.className should be former. Threw exception TypeError: null is not an object (evaluating 'datalist.className')
>  FAIL document.getElementById("text").list should be [object HTMLDataListElement]. Was null.

Chris, is this fine although result mentioned FAIL ?

[Chris Dumez]

(In reply to comment #2)
> (From update of attachment 208800 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=208800&action=review
> 
> > LayoutTests/platform/efl/fast/forms/datalist/input-list-expected.txt:10
> > -FAIL datalist.className should be former. Threw exception TypeError: 'null' is not an object (evaluating 'datalist.className')
> > +FAIL datalist.className should be former. Threw exception TypeError: null is not an object (evaluating 'datalist.className')
> >  FAIL document.getElementById("text").list should be [object HTMLDataListElement]. Was null.
> 
> Chris, is this fine although result mentioned FAIL ?

It was already FAIL before so it is not any worse IMO.

[Chris Dumez]

Comment on attachment 208800 [details]
Patch

Ryuan wants to skip it, which is indeed the right approach IMO. So r-.

[Santosh Mahto]

(In reply to comment #4)
> (From update of attachment 208800 [details])
> Ryuan wants to skip it, which is indeed the right approach IMO. So r-.

I doubt about skipping this test.
This test has test cases for all input type="test/color/speech/email/range/search/url/date/time/week/..........."

in efl datalist is supported only for input type="email/range/search/url"

So this test is used as test for input type="email/range/search/url". for these type it succeeds.For other type we have FAIL as xx-expected.txt.

WhenEver we implement datalist UI for new type we just need to change  expected output from FAIL to PASS.

[Chris Dumez]

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 208800 [details] [details])
> > Ryuan wants to skip it, which is indeed the right approach IMO. So r-.
> 
> I doubt about skipping this test.
> This test has test cases for all input type="test/color/speech/email/range/search/url/date/time/week/..........."
> 
> in efl datalist is supported only for input type="email/range/search/url"
> 
> So this test is used as test for input type="email/range/search/url". for these type it succeeds.For other type we have FAIL as xx-expected.txt.
> 
> WhenEver we implement datalist UI for new type we just need to change  expected output from FAIL to PASS.

Right, I overlooked that it was testing all the input types. Then, rebaselining makes sense since we can only improve the situation gradually. I say let's land this. BTW, this is a gardening patch so you don't need review. Just get rid of the "reviewed by" line in the Changelog and add "Unreviewed" to your Changelog.

[Ryuan Choi]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (From update of attachment 208800 [details] [details] [details])
> > > Ryuan wants to skip it, which is indeed the right approach IMO. So r-.
> > 
> > I doubt about skipping this test.
> > This test has test cases for all input type="test/color/speech/email/range/search/url/date/time/week/..........."
> > 
> > in efl datalist is supported only for input type="email/range/search/url"
> > 
> > So this test is used as test for input type="email/range/search/url". for these type it succeeds.For other type we have FAIL as xx-expected.txt.
> > 
> > WhenEver we implement datalist UI for new type we just need to change  expected output from FAIL to PASS.
> 
> Right, I overlooked that it was testing all the input types. Then, rebaselining makes sense since we can only improve the situation gradually. I say let's land this. BTW, this is a gardening patch so you don't need review. Just get rid of the "reviewed by" line in the Changelog and add "Unreviewed" to your Changelog.

OK, I just focused "FAIL" and does not consider gradual improvement.

I thought that skipped test is easier to find what is missing for EFL port.

[Santosh Mahto]

Created attachment 208903 [details]
Patch

[Chris Dumez]

Comment on attachment 208903 [details]
Patch

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

> LayoutTests/ChangeLog:3
> +        <https://webkit.org/b/119784> [EFL] fast/forms/datalist/input-list.html is failing

This does not look right. Please regenerate the Changelog.

> LayoutTests/ChangeLog:5
> +        [EFL] Unreviwed gardening.

"Unreviewed"

[Santosh Mahto]

Created attachment 208909 [details]
Patch

[Chris Dumez]

Comment on attachment 208909 [details]
Patch

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

> LayoutTests/ChangeLog:5
> +        [EFL] Unreviewed gardening.

No "[EFL]" as this is not a bug title ("Unreviewed EFL gardening, blablabla." would be OK though). Also please add a quick description of your change. We cannot land changes without Changelog as per WK policy.

[Santosh Mahto]

Created attachment 208917 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 208917 [details]
Patch

Clearing flags on attachment: 208917

Committed r154177: <http://trac.webkit.org/changeset/154177>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.