[webkit-reviews] review granted: [Bug 60262] Eliminate WebCore/dom/InputElement.{cpp, h} : [Attachment 92538] Patch 3

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Kent Tamura <tkent at chromium.org>'s
request for review:
Bug 60262: Eliminate WebCore/dom/InputElement.{cpp,h}
https://bugs.webkit.org/show_bug.cgi?id=60262

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=92538&action=review

> Source/WebCore/html/HTMLInputElement.cpp:518
> +	   m_value =  sanitizeValue(fastGetAttribute(valueAttr));

Nit: Two spaces after =.

> Source/WebCore/html/HTMLInputElement.cpp:649
> +	   int maxLength = attr->isNull() ? maximumLength :
attr->value().toInt();
> +	   if (maxLength <= 0 || maxLength > maximumLength)
> +	       maxLength = maximumLength;
> +	   int oldMaxLength = m_maxLength;
> +	   m_maxLength = maxLength;
> +	   if (oldMaxLength != maxLength)
> +	       updateValueIfNeeded();

I would have preferred to keep this in a helper function.

> Source/WebCore/html/HTMLInputElement.cpp:655
> +	   if (renderer())
> +	       renderer()->setNeedsLayoutAndPrefWidthsRecalc();

Maybe we need to call setNeedsStyleRecalc here as well because we could have a
style rule that matches size attribute such as input[size=5]. Of course, that
would be a separate patch.

> Source/WebCore/html/HTMLInputElement.cpp:1519
> +static inline const AtomicString& formatCodes()
> +{
> +    DEFINE_STATIC_LOCAL(AtomicString, codes, ("AaNnXxMm"));
> +    return codes;
> +}

This entire section needs to be cleaned up at some point :(