Bug 111319 – Add clear button to date/time input types

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Keishi Hattori]

Replace spin buttons with clear button for input types date/week/month/datetime-local

[Keishi Hattori]

Created attachment 191250 [details]
Patch

[WebKit Review Bot]

Attachment 191250 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-clearbutton-change-and-input-events-expected.txt', u'LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-clearbutton-change-and-input-events.html', u'LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-mouse-events-expected.txt', u'LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-mouse-events.html', u'LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-spinbutton-change-and-input-events-expected.txt', u'LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-spinbutton-change-and-input-events.html', u'LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-clearbutton-change-and-input-events-expected.txt', u'LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-clearbutton-change-and-input-events.html', u'LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-mouse-events-expected.txt', u'LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-mouse-events.html', u'LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-spinbutton-change-and-input-events.html', u'LayoutTests/fast/forms/month-multiple-fields/month-multiple-fields-clearbutton-change-and-input-events-expected.txt', u'LayoutTests/fast/forms/month-multiple-fields/month-multiple-fields-clearbutton-change-and-input-events.html', u'LayoutTests/fast/forms/month-multiple-fields/month-multiple-fields-mouse-events-expected.txt', u'LayoutTests/fast/forms/month-multiple-fields/month-multiple-fields-mouse-events.html', u'LayoutTests/fast/forms/month-multiple-fields/month-multiple-fields-spinbutton-change-and-input-events.html', u'LayoutTests/fast/forms/resources/common-clearbutton-change-and-input-events.js', u'LayoutTests/fast/forms/week-multiple-fields/week-multiple-fields-clearbutton-change-and-input-events-expected.txt', u'LayoutTests/fast/forms/week-multiple-fields/week-multiple-fields-clearbutton-change-and-input-events.html', u'LayoutTests/fast/forms/week-multiple-fields/week-multiple-fields-mouse-events-expected.txt', u'LayoutTests/fast/forms/week-multiple-fields/week-multiple-fields-mouse-events.html', u'LayoutTests/fast/forms/week-multiple-fields/week-multiple-fields-spinbutton-change-and-input-events.html', u'LayoutTests/platform/chromium-mac/fast/forms/date/date-appearance-basic-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/date/date-appearance-l10n-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/date/date-appearance-pseudo-elements-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/datetimelocal/datetimelocal-appearance-basic-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/datetimelocal/datetimelocal-appearance-l10n-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/month/month-appearance-basic-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/month/month-appearance-l10n-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/month/month-appearance-pseudo-elements-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/week/week-appearance-basic-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/week/week-appearance-pseudo-elements-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearance-rtl-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearance-with-scroll-bar-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-appearance-locale-hebrew-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-appearance-rtl-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-appearance-with-scroll-bar-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-appearance-rtl-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-appearance-with-scroll-bar-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-appearance-rtl-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-appearance-with-scroll-bar-expected.png', u'LayoutTests/platform/chromium/TestExpectations', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/WebCore.gypi', u'Source/WebCore/css/html.css', u'Source/WebCore/html/BaseMultipleFieldsDateAndTimeInputType.cpp', u'Source/WebCore/html/BaseMultipleFieldsDateAndTimeInputType.h', u'Source/WebCore/html/DateTimeFieldsState.cpp', u'Source/WebCore/html/DateTimeFieldsState.h', u'Source/WebCore/html/HTMLInputElement.cpp', u'Source/WebCore/html/InputType.cpp', u'Source/WebCore/html/InputType.h', u'Source/WebCore/html/shadow/ClearButtonElement.cpp', u'Source/WebCore/html/shadow/ClearButtonElement.h', u'Source/WebCore/html/shadow/DateTimeEditElement.cpp', u'Source/WebCore/html/shadow/DateTimeEditElement.h', u'Source/WebCore/html/shadow/DateTimeFieldElement.cpp', u'Source/WebCore/html/shadow/DateTimeFieldElement.h']" exit_code: 1
LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-appearance-with-scroll-bar-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Source/WebCore/html/DateTimeFieldsState.cpp:113:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/html/DateTimeFieldsState.cpp:114:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 3 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 191250 [details]
Patch

Attachment 191250 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/16952006

New failing tests:
fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-clearbutton-change-and-input-events.html
fast/forms/month-multiple-fields/month-multiple-fields-clearbutton-change-and-input-events.html
fast/forms/week-multiple-fields/week-multiple-fields-clearbutton-change-and-input-events.html
fast/forms/date-multiple-fields/date-multiple-fields-clearbutton-change-and-input-events.html

[Keishi Hattori]

Created attachment 191307 [details]
Patch

[WebKit Review Bot]

Attachment 191307 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-clearbutton-change-and-input-events-expected.txt', u'LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-clearbutton-change-and-input-events.html', u'LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-mouse-events-expected.txt', u'LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-mouse-events.html', u'LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-spinbutton-change-and-input-events-expected.txt', u'LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-spinbutton-change-and-input-events.html', u'LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-clearbutton-change-and-input-events-expected.txt', u'LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-clearbutton-change-and-input-events.html', u'LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-mouse-events-expected.txt', u'LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-mouse-events.html', u'LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-spinbutton-change-and-input-events.html', u'LayoutTests/fast/forms/month-multiple-fields/month-multiple-fields-clearbutton-change-and-input-events-expected.txt', u'LayoutTests/fast/forms/month-multiple-fields/month-multiple-fields-clearbutton-change-and-input-events.html', u'LayoutTests/fast/forms/month-multiple-fields/month-multiple-fields-mouse-events-expected.txt', u'LayoutTests/fast/forms/month-multiple-fields/month-multiple-fields-mouse-events.html', u'LayoutTests/fast/forms/month-multiple-fields/month-multiple-fields-spinbutton-change-and-input-events.html', u'LayoutTests/fast/forms/resources/common-clearbutton-change-and-input-events.js', u'LayoutTests/fast/forms/week-multiple-fields/week-multiple-fields-clearbutton-change-and-input-events-expected.txt', u'LayoutTests/fast/forms/week-multiple-fields/week-multiple-fields-clearbutton-change-and-input-events.html', u'LayoutTests/fast/forms/week-multiple-fields/week-multiple-fields-mouse-events-expected.txt', u'LayoutTests/fast/forms/week-multiple-fields/week-multiple-fields-mouse-events.html', u'LayoutTests/fast/forms/week-multiple-fields/week-multiple-fields-spinbutton-change-and-input-events.html', u'LayoutTests/platform/chromium-mac/fast/forms/date/date-appearance-basic-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/date/date-appearance-l10n-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/date/date-appearance-pseudo-elements-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/datetimelocal/datetimelocal-appearance-basic-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/datetimelocal/datetimelocal-appearance-l10n-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/month/month-appearance-basic-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/month/month-appearance-l10n-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/month/month-appearance-pseudo-elements-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/week/week-appearance-basic-expected.png', u'LayoutTests/platform/chromium-mac/fast/forms/week/week-appearance-pseudo-elements-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearance-rtl-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearance-with-scroll-bar-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-appearance-locale-hebrew-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-appearance-rtl-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-appearance-with-scroll-bar-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-appearance-rtl-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-appearance-with-scroll-bar-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-appearance-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-appearance-rtl-expected.png', u'LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-appearance-with-scroll-bar-expected.png', u'LayoutTests/platform/chromium/TestExpectations', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/WebCore.gypi', u'Source/WebCore/css/html.css', u'Source/WebCore/html/BaseMultipleFieldsDateAndTimeInputType.cpp', u'Source/WebCore/html/BaseMultipleFieldsDateAndTimeInputType.h', u'Source/WebCore/html/DateTimeFieldsState.cpp', u'Source/WebCore/html/DateTimeFieldsState.h', u'Source/WebCore/html/HTMLInputElement.cpp', u'Source/WebCore/html/InputType.cpp', u'Source/WebCore/html/InputType.h', u'Source/WebCore/html/shadow/ClearButtonElement.cpp', u'Source/WebCore/html/shadow/ClearButtonElement.h', u'Source/WebCore/html/shadow/DateTimeEditElement.cpp', u'Source/WebCore/html/shadow/DateTimeEditElement.h', u'Source/WebCore/html/shadow/DateTimeFieldElement.cpp', u'Source/WebCore/html/shadow/DateTimeFieldElement.h']" exit_code: 1
LayoutTests/platform/chromium-mac/platform/chromium/fast/forms/suggestion-picker/datetimelocal-suggestion-picker-appearance-with-scroll-bar-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kent Tamura]

Comment on attachment 191307 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        We are going to replace the spin buttons in input types date/week/month/datetime-local with a clear value button.

I don't think the replacement is reasonable. Why don't you just add the clear button?
The input[type=time] exception is bad for consistency, and we want the clear button for input[type=time] too.

> Source/WebCore/html/BaseMultipleFieldsDateAndTimeInputType.cpp:499
> +    if (element()->isRequired() || m_dateTimeEditElement->valueAsDateTimeFieldsState().isEmpty())

m_dateTimeEditElement->valueAsDateTimeFieldsState().isEmpty() is equivalent to !m_dateTimeEditElement->anyEditableFieldsHaveValues()

> Source/WebCore/html/shadow/ClearButtonElement.h:33
> +class ClearButtonElement : public HTMLDivElement {

We had better re-use SearchFieldCancelButtonElement.

[Keishi Hattori]

(In reply to comment #6)
> (From update of attachment 191307 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=191307&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        We are going to replace the spin buttons in input types date/week/month/datetime-local with a clear value button.
> 
> I don't think the replacement is reasonable. Why don't you just add the clear button?
> The input[type=time] exception is bad for consistency, and we want the clear button for input[type=time] too.
ok I'm adding the clear button to all date/time types.

> > Source/WebCore/html/BaseMultipleFieldsDateAndTimeInputType.cpp:499
> > +    if (element()->isRequired() || m_dateTimeEditElement->valueAsDateTimeFieldsState().isEmpty())
> 
> m_dateTimeEditElement->valueAsDateTimeFieldsState().isEmpty() is equivalent to !m_dateTimeEditElement->anyEditableFieldsHaveValues()
Done.
> > Source/WebCore/html/shadow/ClearButtonElement.h:33
> > +class ClearButtonElement : public HTMLDivElement {
> 
> We had better re-use SearchFieldCancelButtonElement.
Done.

[Keishi Hattori]

Created attachment 191487 [details]
Patch

[Keishi Hattori]

I had to remove the image files because the size went over 2MB.

[Early Warning System Bot]

Comment on attachment 191487 [details]
Patch

Attachment 191487 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17055092

[Early Warning System Bot]

Comment on attachment 191487 [details]
Patch

Attachment 191487 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17053154

[Build Bot]

Comment on attachment 191487 [details]
Patch

Attachment 191487 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17046185

[Build Bot]

Comment on attachment 191487 [details]
Patch

Attachment 191487 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17071016

[WebKit Review Bot]

Comment on attachment 191487 [details]
Patch

Attachment 191487 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17013079

New failing tests:
platform/chromium/fast/forms/suggestion-picker/time-suggestion-picker-appearance-locale-hebrew.html
fast/forms/time/time-appearance-basic.html
fast/forms/time/time-appearance-pseudo-elements.html
fast/css/text-input-with-webkit-border-radius.html
touchadjustment/search-cancel.html
platform/chromium/fast/forms/suggestion-picker/time-suggestion-picker-appearance-rtl.html
fast/css/text-overflow-input.html
fast/repaint/search-field-cancel.html
fast/speech/input-appearance-searchandspeech.html
platform/chromium/fast/forms/suggestion-picker/time-suggestion-picker-appearance.html
platform/chromium/fast/forms/suggestion-picker/time-suggestion-picker-appearance-with-scroll-bar.html
fast/css/input-search-padding.html
fast/replaced/width100percent-searchfield.html

[Kent Tamura]

Comment on attachment 191487 [details]
Patch

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

> Source/WebCore/ChangeLog:60
> +        * html/shadow/ClearButtonElement.cpp: Added.

Adding ClearButtonElement and updating input[type=search] code should be a separated patch, and I think we can avoid rebaseline for existing type=search tests.

> Source/WebCore/css/html.css:458
> -input[type="search"]::-webkit-search-cancel-button {
> +input::-webkit-clear-button {
>      -webkit-appearance: searchfield-cancel-button;
>      display: block;
>      -webkit-box-flex: 0;
>      -webkit-user-modify: read-only !important;
> +    margin-left: 2px;

We don't need to add the margin for input[type=search]. So, we should

input::-webkit-clear-button {
  ...
  display: inline-block; /* Multiple fields UI requires inline-block. See Bug 110974  */
  ....
  margin-left: 2px;
}

input[type="search"]::-webkit-clear-button {
    margin-left: 0;
}

> Source/WebCore/html/SearchInputType.cpp:200
> +    element()->focus();
> +    element()->select();

Possible use-after-free for 'this' by focus events.

> Source/WebCore/html/SearchInputType.cpp:212
> +    element()->setValueForUser("");
> +    updateClearButtonVisibility();
> +    element()->onSearch();

Possible use-after-free for 'this' by 'change' 'input' events.

[Keishi Hattori]

Created attachment 191694 [details]
Patch

[Kent Tamura]

Comment on attachment 191694 [details]
Patch

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

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:9576
> -		7EA30F6716DFFE7500257D0B /* JSWebGLCompressedTextureATC.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = JSWebGLCompressedTextureATC.cpp; path = JSWebGLCompressedTextureATC.cpp; sourceTree = "<group>"; };
> -		7EA30F6816DFFE7500257D0B /* JSWebGLCompressedTextureATC.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = JSWebGLCompressedTextureATC.h; path = JSWebGLCompressedTextureATC.h; sourceTree = "<group>"; };
> +		7EA30F6716DFFE7500257D0B /* JSWebGLCompressedTextureATC.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSWebGLCompressedTextureATC.cpp; sourceTree = "<group>"; };
> +		7EA30F6816DFFE7500257D0B /* JSWebGLCompressedTextureATC.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSWebGLCompressedTextureATC.h; sourceTree = "<group>"; };

You don't need to update these lines

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:26694
> +				C37DF8F016E746710079A0EB /* ClearButtonElement.h in Headers */,

Put this line in a sorted position.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:29897
> +				C37DF8EF16E746710079A0EB /* ClearButtonElement.cpp in Sources */,

Ditto.

> Source/WebCore/html/BaseMultipleFieldsDateAndTimeInputType.cpp:486
> +    element()->setValue("", DispatchInputAndChangeEvent);
> +    updateClearButtonVisibility();

Possible use-after-free for 'this' because change/input event handlers can change the input type.
We had better have HTMLInputElement::updateClearButtonVisibility and InputType::updateClearButtonVisibility, and this function would be something like:

RefPtr<HTMLInputElement> input(element());
input->setValue(...);
input->updateClearButtonVisibility();

> Source/WebCore/html/BaseMultipleFieldsDateAndTimeInputType.cpp:512
> +void BaseMultipleFieldsDateAndTimeInputType::hideClearButton()
> +{
> +    if (!m_clearButton)
> +        return;
> +    m_clearButton->setInlineStyleProperty(CSSPropertyVisibility, CSSValueHidden);
> +}
> +
> +void BaseMultipleFieldsDateAndTimeInputType::showClearButton()
> +{
> +    if (!m_clearButton)
> +        return;
> +    m_clearButton->removeInlineStyleProperty(CSSPropertyVisibility);
> +}

These functions are used only by updateClearButtonVisiblity. We can fold them into updateClearButtonVisibility and remove the functions.

> Source/WebCore/html/shadow/ClearButtonElement.cpp:1
> +#include "config.h"

Need a copyright header.

> Source/WebCore/html/shadow/ClearButtonElement.cpp:28
> +const AtomicString& ClearButtonElement::shadowPseudoId() const
> +{
> +    DEFINE_STATIC_LOCAL(AtomicString, pseudoId, ("-webkit-clear-button", AtomicString::ConstructFromLiteral));
> +    return pseudoId;
> +}

nit: You don't need to override shadowPseudoId nowadays. You can call setPseudo in the constructor.

> Source/WebCore/html/shadow/ClearButtonElement.cpp:41
> +    if (m_capturing) {

We prefer early return.

> Source/WebCore/html/shadow/ClearButtonElement.h:1
> +#ifndef ClearButtonElement_h

Need a copyright header.

> Source/WebCore/html/shadow/ClearButtonElement.h:20
> +    virtual void releaseCapture();

virtual is not needed?

> Source/WebCore/html/shadow/ClearButtonElement.h:23
> +    virtual void defaultEventHandler(Event*);

It seems we can make this private.

[Keishi Hattori]

Created attachment 191908 [details]
Patch

[WebKit Review Bot]

Comment on attachment 191908 [details]
Patch

Attachment 191908 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17000373

New failing tests:
platform/chromium/fast/forms/suggestion-picker/time-suggestion-picker-appearance-locale-hebrew.html
fast/forms/time/time-appearance-basic.html
fast/forms/time/time-appearance-pseudo-elements.html
platform/chromium/fast/forms/suggestion-picker/time-suggestion-picker-appearance.html
platform/chromium/fast/forms/suggestion-picker/time-suggestion-picker-appearance-rtl.html
platform/chromium/fast/forms/suggestion-picker/time-suggestion-picker-appearance-with-scroll-bar.html

[Kent Tamura]

Comment on attachment 191908 [details]
Patch

Looks good.  Please address the cr-linux EWS failures.

[Keishi Hattori]

Created attachment 191946 [details]
Patch

[WebKit Review Bot]

Comment on attachment 191946 [details]
Patch

Clearing flags on attachment: 191946

Committed r145055: <http://trac.webkit.org/changeset/145055>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Roger Fong]

Hi, immediately after this patch, many of the layout tests on Window started failing on the bots. 
Namely, it looks like all the dom/html/level2/html/ tests are crashing.

I for some reason cannot reproduce locally and am not sure why this is happening.
Can we temporarily roll this out just so I can confirm whether or not it's really this patch that's causing the issue?
Much appreciated.

[Roger Fong]

(In reply to comment #24)
> Hi, immediately after this patch, many of the layout tests on Window started failing on the bots. 
> Namely, it looks like all the dom/html/level2/html/ tests are crashing.
> 
> I for some reason cannot reproduce locally and am not sure why this is happening.
> Can we temporarily roll this out just so I can confirm whether or not it's really this patch that's causing the issue?
> Much appreciated.

Looks like a different JSC patch fixed the issue. nvm!