Bug 98390 - Update some picker tests for input[type=date]
Summary: Update some picker tests for input[type=date]
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords:
Depends on:
Blocks: 98226
  Show dependency treegraph
 
Reported: 2012-10-04 02:44 PDT by Kent Tamura
Modified: 2012-10-04 21:45 PDT (History)
5 users (show)

See Also:


Attachments
WIP (24.07 KB, patch)
2012-10-04 04:39 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch (24.47 KB, patch)
2012-10-04 07:53 PDT, Kent Tamura
dbates: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2012-10-04 02:44:56 PDT
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.
Comment 1 Kent Tamura 2012-10-04 04:39:28 PDT
Created attachment 167074 [details]
WIP
Comment 2 Kent Tamura 2012-10-04 07:53:40 PDT
Created attachment 167103 [details]
Patch
Comment 3 Daniel Bates 2012-10-04 11:27:47 PDT
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.
Comment 4 Kent Tamura 2012-10-04 16:40:57 PDT
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.
Comment 5 Kent Tamura 2012-10-04 16:50:07 PDT
Committed r130433: <http://trac.webkit.org/changeset/130433>
Comment 6 Kent Tamura 2012-10-04 21:45:20 PDT
(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.