[webkit-reviews] review granted: [Bug 25552] Support for HTML5 Forms "pattern" attribute : [Attachment 33389] Proposed patch, rev. 2

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Michelangelo De Simone
<micdesim at gmail.com>'s request for review:
Bug 25552: Support for HTML5 Forms "pattern" attribute
https://bugs.webkit.org/show_bug.cgi?id=25552

Attachment 33389: Proposed patch, rev. 2
https://bugs.webkit.org/attachment.cgi?id=33389&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    const AtomicString& l_pattern = getAttribute(patternAttr);

What's the "l_" prefix here? We don't use underscores in variable names. You
can just name this local variable pattern, I believe. It will shadow the
pattern() function, but that should be fine; you can always use this->pattern()
if you need to get at the pattern function.

> +    if (l_pattern.isEmpty() || value().isEmpty())

This function calls value(), a virtual function, three times. I would suggest
doing this at the start of the function:

    String value = this->value();

And then use the local value throughout.

I'm going to say r=me, but we probably do want to fix the l_pattern variable
name.