I would like to set SpinButtonElement free from HTMLInputELement, because: 1. Concept and functionality of spin button is independent from the "input" element. 2. ease of mountainous: HTMLInputElement.{cpp,h} changes don't affect to SpinButtonElement 3. faster build time when HTMLInputElement.h changed. Frequency of HTMLInputElement.h changes is more often than SpinButtonElement.h, because HTMLInputElement has lot of methods for various input types.
I also move code of SpinButtonElement to separate file. Since multiple fields time input UI, SpinButtonElement is not only text field inner element but also inner element of multiple fields time input UI.
Created attachment 158749 [details] Patch 1
Comment on attachment 158749 [details] Patch 1 Attachment 158749 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13517286 New failing tests: fast/forms/number/number-change-type-on-focus.html
Created attachment 158757 [details] Archive of layout-test-results from gce-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 158768 [details] Patch 2
Comment on attachment 158768 [details] Patch 2 The patch doesn't have r? flag, but I have a comment. I recommend to split the patch into three pieces. 1. Add SpinButtonElement.{cpp.h} to build files, and remove unnecessary code 2. Rename StepActionHandler to SpinButtonOwner 3. Remaining Reviewers can set r+ for 1 and 2 easily because they are mechanical changes. Reviewers need to review 3 and it should be smaller in order to concentrate on it easily. Actually, your first patch had use-after-free.
(In reply to comment #6) > Reviewers need to review 3 need to review 3 carefully
Comment on attachment 158768 [details] Patch 2 Could you review this patch? Thanks in advance.
Comment on attachment 158768 [details] Patch 2 r- because of comment #6
Created attachment 159064 [details] Patch 3
Comment on attachment 159064 [details] Patch 3 Could you review this patch? Thanks in advance.
Comment on attachment 159064 [details] Patch 3 View in context: https://bugs.webkit.org/attachment.cgi?id=159064&action=review > Source/WebCore/ChangeLog:40 > + * html/TextFieldInputType.cpp: > + (WebCore::TextFieldInputType::focusAndSelectSpinButtonOwner): > + (WebCore::TextFieldInputType::shouldSpinButtonRespondToMouseEvents): > + (WebCore::TextFieldInputType::shouldSpinButtonRespondToWheelEvents): > + * html/TextFieldInputType.h: > + (TextFieldInputType): > + * html/TimeInputType.cpp: > + (WebCore::TimeInputType::DateTimeEditControlOwnerImpl::focusAndSelectEditControlOwner): > + (WebCore::TimeInputType::DateTimeEditControlOwnerImpl::isEditControlOwnerFocused): > + * html/TimeInputType.h: > + (DateTimeEditControlOwnerImpl): > + * html/shadow/DateTimeEditElement.cpp: > + (WebCore::DateTimeEditElement::focusAndSelectSpinButtonOwner): > + (WebCore::DateTimeEditElement::shouldSpinButtonRespondToMouseEvents): > + (WebCore::DateTimeEditElement::shouldSpinButtonRespondToWheelEvents): > + * html/shadow/DateTimeEditElement.h: > + (EditControlOwner): > + (DateTimeEditElement): > + * html/shadow/SpinButtonElement.cpp: > + (WebCore::SpinButtonElement::defaultEventHandler): > + (WebCore::SpinButtonElement::forwardEvent): > + (WebCore::SpinButtonElement::willRespondToMouseMoveEvents): > + (WebCore::SpinButtonElement::willRespondToMouseClickEvents): > + (WebCore::SpinButtonElement::step): > + (WebCore::SpinButtonElement::shouldRespondToMouseEvents): > + * html/shadow/SpinButtonElement.h: > + (SpinButtonOwner): > + (SpinButtonElement): You had better add per-function comments, especially to existing functions.
Created attachment 159330 [details] Patch 4
Comment on attachment 159330 [details] Patch 4 Clearing flags on attachment: 159330 Committed r125997: <http://trac.webkit.org/changeset/125997>
All reviewed patches have been landed. Closing bug.