[webkit-reviews] review granted: [Bug 21248] Support placeholder on textarea : [Attachment 34534] Proposed patch (rev.4)

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted TAMURA, Kent <tkent at chromium.org>'s
request for review:
Bug 21248: Support placeholder on textarea
https://bugs.webkit.org/show_bug.cgi?id=21248

Attachment 34534: Proposed patch (rev.4)
https://bugs.webkit.org/attachment.cgi?id=34534&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
This looks great.  I'm just sad we can's share implementations:

+    static void dispatchFocusEvent(InputElement*, Element*);
+    static void dispatchBlurEvent(InputElement*, Element*);
+    static bool placeholderShouldBeVisible(const InputElement*, const
Element*);
+    static void updatePlaceholderVisibility(InputElement*, Element*, bool
placeholderValueChanged = false);

We could put those on HTMLFormControlElement... or on a new
HTMLTextControlElement subclass.  Or on some class which both of these used
(since "having" placeholder text does not necessarily mean you need to "be" a
classs which can have placehodler text.  We could use 'has a' instead of 'is a'
to share code.)

I'm going to approve this as is.  I would like you to consider a follow-up
patch to share more of this code.