[webkit-reviews] review denied: [Bug 95168] [Forms] Shift+Tab should focus the last field of multiple fields time input UI : [Attachment 161439] Patch 5

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied yosin at chromium.org's request for
review:
Bug 95168: [Forms] Shift+Tab should focus the last field of multiple fields
time input UI
https://bugs.webkit.org/show_bug.cgi?id=95168

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

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


> Source/WebCore/ChangeLog:49
> +	   (WebCore::DateTimeEditElement::didFieldBlur): Added for calling
DateTimeEditControlOwner::didFieldBlur().
> +	   (WebCore::DateTimeEditElement::didFieldFocus): Added for calling
DateTimeEditControlOwner::didFieldFocus().

Need to update the comments.

> Source/WebCore/ChangeLog:65
> +	   (EditControlOwner): Added didFieldBlur() and didFieldFocus()
function declarations.

Need to update the comment.
Also, you should mention removed functions and their reasons.

> Source/WebCore/html/shadow/DateTimeFieldElement.h:54
> +	   virtual void didFocusFromField() = 0;

"focus from field" sounds strange.  It should be didFocusOnField.

>
LayoutTests/fast/forms/time-multiple-fields/time-multiple-fields-blur-and-focus
-events.html:50
> +    shouldBeEqualToString('keyDown("\t"); state()', 'blur=0 focus=1');
> +    shouldBeEqualToString('keyDown("\t"); state()', 'blur=0 focus=1');
> +    shouldBeEqualToString('keyDown("\t"); state()', 'blur=0 focus=1');

Oh, I thought focus and blur were dispatched in a case of focus change between
internal fields.  This result is nice.