[Webkit-unassigned] [Bug 37881] Numerous style violations in HTMLInputElement.cpp

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=37881


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #53864|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #2 from Darin Adler <darin at apple.com>  2010-04-20 15:02:48 PST ---
(From update of attachment 53864)
Since this is all about style I was picky about style.

> +         RegularExpression patternRegExp(pattern, TextCaseSensitive);
> +        int matchLength = 0;

Looks like the patternRegExp line is incorrectly indented.

> -            if (focusedInput->inputType() == RADIO && focusedInput->form() == form() &&
> -                focusedInput->name() == name())
> +            if (focusedInput->inputType() == RADIO && focusedInput->form() == form() 
> +                && focusedInput->name() == name())
>                  return false;

I think this would be better if it was just all on one long line.

> -    } else if (attr->name() == usemapAttr ||
> -               attr->name() == accesskeyAttr) {
> +    } else if (attr->name() == usemapAttr 
> +            || attr->name() == accesskeyAttr) {

I think this would be better if it was just all on one long line.

> +    } else if (attr->name() == onsearchAttr) {
> +    // Search field and slider attributes all just cause updateFromElement to be called through style recalcing.
>          setAttributeEventListener(eventNames().searchEvent, createAttributeEventListener(this, attr));

I think the comment should be indented at the same level as the code below it.

> -void HTMLInputElement::setIndeterminate(bool _indeterminate)
> +void HTMLInputElement::setIndeterminate(bool indet)

We normally use words, not abbreviations. I think "indet" is unclear. Maybe
"newValue" would be better.

Otherwise looks fine. review- because you should fix at least some, preferably
all, of what I mentioned above.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.