[webkit-reviews] review denied: [Bug 37881] Numerous style violations in HTMLInputElement.cpp : [Attachment 53864] Corrections for all style test failures.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Apr 20 15:02:47 PDT 2010


Darin Adler <darin at apple.com> has denied Ray <Raymond.Rischpater at Nokia.com>'s
request for review:
Bug 37881: Numerous style violations in HTMLInputElement.cpp
https://bugs.webkit.org/show_bug.cgi?id=37881

Attachment 53864: Corrections for all style test failures.
https://bugs.webkit.org/attachment.cgi?id=53864&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
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.


More information about the webkit-reviews mailing list