[webkit-reviews] review denied: [Bug 99537] Hide popup while transitioning from the suggestion picker to the calendar picker : [Attachment 169128] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Keishi Hattori
<keishi at webkit.org>'s request for review:
Bug 99537: Hide popup while transitioning from the suggestion picker to the
calendar picker
https://bugs.webkit.org/show_bug.cgi?id=99537

Attachment 169128: Patch
https://bugs.webkit.org/attachment.cgi?id=169128&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=169128&action=review


> LayoutTests/ChangeLog:9
> +	   Use didOpenPicker event instead of resize event. Also refactoring
> +	   tests to share the same structure.

Would you separate it into two patches please?

> LayoutTests/ChangeLog:37
> +	   *
platform/chromium/fast/forms/color/color-suggestion-picker-common.js: Added.

We usually put such helper library into resources/ subdirectory.

> LayoutTests/platform/chromium/TestExpectations:3904
> +webkit.org/b/99291
platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance.html [
Pass ImageOnlyFailure ]
> +webkit.org/b/99291
platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-step.ht
ml [ Pass ImageOnlyFailure ]

This file already has lines for them.

> LayoutTests/platform/chromium/TestExpectations:3910
> +webkit.org/b/99291
platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearanc
e.html [ Pass ImageOnlyFailure ]
> +webkit.org/b/99291
platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearanc
e-with-scroll-bar.html [ Pass ImageOnlyFailure ]
> +webkit.org/b/99291
platform/chromium/fast/forms/suggestion-picker/time-suggestion-picker-appearanc
e.html [ Pass ImageOnlyFailure ]
> +webkit.org/b/99291
platform/chromium/fast/forms/suggestion-picker/time-suggestion-picker-appearanc
e-with-scroll-bar.html [ Pass ImageOnlyFailure ]

Ditto.

>
LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-key-op
erations-expected.txt:53
> -PASS focusedElement() is ".today-button[value=<<CalendarToday>>]"
> -PASS focusedElement() is ".clear-button[value=<<CalendarClear>>]"
> +FAIL focusedElement() should be .today-button[value=Today]. Was
.today-button[value=<<CalendarToday>>].
> +FAIL focusedElement() should be .clear-button[value=Clear]. Was
.clear-button[value=<<CalendarClear>>].
>  PASS focusedElement() is ".year-month-button[value=<<]"
> -PASS focusedElement() is ".clear-button[value=<<CalendarClear>>]"
> +FAIL focusedElement() should be .clear-button[value=Clear]. Was
.clear-button[value=<<CalendarClear>>].

Is this an expected change?

>
LayoutTests/platform/chromium/fast/forms/suggestion-picker/time-suggestion-pick
er-key-operations.html:90
> -    popupWindow.addEventListener("resize", test1, false);
> +    popupWindow.addEventListener("didOpenPicker", test1, false);

Don't you use the callback argument of openPicker()?

>
LayoutTests/platform/chromium/fast/forms/suggestion-picker/time-suggestion-pick
er-key-operations.html:145
>      openPicker(document.getElementById('time'));
> -    popupWindow.addEventListener("resize", test2, false);
> +    popupWindow.addEventListener("didOpenPicker", test2, false);

ditto.