Bug 98390 – Update some picker tests for input[type=date]

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Kent Tamura]

fast/forms/date/calendar-picker-appearance-pre-100.html [ Timeout ] 
  fast/forms/date/calendar-picker-appearance.html [ Timeout ] 
  fast/forms/date/calendar-picker-key-operations.html [ Timeout ] 
  fast/forms/date/calendar-picker-mouse-operations.html [ Timeout ] 
  fast/forms/date/calendar-picker-with-step.html [ Timeout ] 
  platform/chromium/fast/forms/date/date-suggestion-picker-appearance-rtl.html [ Timeout ] 
  platform/chromium/fast/forms/date/date-suggestion-picker-appearance-with-scroll-bar.html [ Timeout ] 
  platform/chromium/fast/forms/date/date-suggestion-picker-appearance.html [ Timeout ] 
  platform/chromium/fast/forms/date/date-suggestion-picker-key-operations.html [ Timeout ] 

This is not a regression.  We need to update these tests.

[Kent Tamura]

Created attachment 167074 [details]
WIP

[Kent Tamura]

Created attachment 167103 [details]
Patch

[Daniel Bates]

Comment on attachment 167103 [details]
Patch

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

This patch looks sane to me.

On another note, I've noticed that the original test code seems to alternate between using single and double quotes. We should pick one style and stick with it for consistency. Also, some tests force a layout before calling openPicker() (using dateInput.offsetTop) and some don't (e.g. LayoutTests/fast/forms/date/calendar-picker-with-step.html). I briefly looked over some of these test files. I take it that it is intentional to force a layout in some tests and not force a layout it others. If I have some time, I'll look to read through the tests in their entirety.

> LayoutTests/ChangeLog:25
> +        ditto.

Nit: ditto => Ditto

> LayoutTests/ChangeLog:27
> +        ditto.

Nit: ditto => Ditto

> LayoutTests/ChangeLog:31
> +        Increase the internal time out becuase it was too short on my machine.

Nit: "time out" => "timeout"

And

becuase => because

> LayoutTests/fast/forms/date/calendar-picker-appearance-pre-100.html:28
> +    dateInput.offsetTop;

I know that the original test had this line. You may want to consider adding an inline comment here that explains that this forces a layout. Even better, explain why we need to force a layout.

> LayoutTests/fast/forms/date/calendar-picker-appearance.html:18
> +    dateInput.offsetTop;

Ditto.

> LayoutTests/platform/chromium/fast/forms/date/date-suggestion-picker-appearance-with-scroll-bar.html:61
> +    dateInput.offsetTop;

Ditto.

> LayoutTests/platform/chromium/fast/forms/date/date-suggestion-picker-appearance.html:41
> +    dateInput.offsetTop;

Ditto.

[Kent Tamura]

Thank you for the comments.

(In reply to comment #3)
> On another note, I've noticed that the original test code seems to alternate between using single and double quotes. We should pick one style and stick with it for consistency. Also, some tests force a layout before calling openPicker() (using dateInput.offsetTop) and some don't (e.g. LayoutTests/fast/forms/date/calendar-picker-with-step.html). I briefly looked over some of these test files. I take it that it is intentional to force a layout in some tests and not force a layout it others. If I have some time, I'll look to read through the tests in their entirety.

Yeah, I also had a concern about single/double quotes.  We had better improve it.
I guess offsetTop hacks were needed for tests which opened a picker before 'load' event, and the hack is not needed because all of tests has been changed so that they open pickers on onload handler.  I'm removing offsetTop in this patch, and will watch bots status.

[Kent Tamura]

Committed r130433: <http://trac.webkit.org/changeset/130433>

[Kent Tamura]

(In reply to comment #4)
> I'm removing offsetTop in this patch, and will watch bots status.

It failed. I added offsetTop again in http://trac.webkit.org/changeset/130458.