[webkit-reviews] review granted: [Bug 27458] Support :default HTML5 CSS selector : [Attachment 34684] Patch v1

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 12 15:05:11 PDT 2009


Darin Adler <darin at apple.com> has granted Michelangelo De Simone
<micdesim at gmail.com>'s request for review:
Bug 27458: Support :default HTML5 CSS selector
https://bugs.webkit.org/show_bug.cgi?id=27458

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

------- Additional Comments from Darin Adler <darin at apple.com>
> +    DEFINE_STATIC_LOCAL(AtomicString, defaultStr, ("default"));

Do we really need the abbreviation "Str" here? I know we can't use default
because it's a reserved word, but how about defaultString or
defaultAtomicString?

> +bool HTMLFormControlElement::isDefaultButtonForForm() const
> +{
> +    if (!isSuccessfulSubmitButton() || !m_form)
> +	   return false;
> +
> +    return m_form->defaultButton() == this;
> +}

I think this would read better in a single line:

    return isSuccessfulSubmitButton() && m_form && m_form->defaultButton() ==
this;

Easier to read!

r=me


More information about the webkit-reviews mailing list