[webkit-reviews] review denied: [Bug 27756] [HTML5][Forms] Support for HTMLInputElement::list and HTMLInputElement::selectedOption : [Attachment 33604] Support for HTMLInputElement::list and HTMLInputElement::selectedOption.

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied TAMURA, Kent <tkent at chromium.org>'s
request for review:
Bug 27756: [HTML5][Forms] Support for HTMLInputElement::list and
HTMLInputElement::selectedOption
https://bugs.webkit.org/show_bug.cgi?id=27756

Attachment 33604: Support for HTMLInputElement::list and
HTMLInputElement::selectedOption.
https://bugs.webkit.org/attachment.cgi?id=33604&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Normally we don't commit commented-out code:
 1738	  //case DATETIME:
 1739	  //case DATE:
 1740	  //case MONTH:
 1741	  //case WEEK:
 1742	  //case TIME:
 1743	  //case DATETIMELOCAL:

Why does this need to be PassRefPtr?
 1727 PassRefPtr<HTMLElement> HTMLInputElement::list()
your'e not returning a newly allocated list.

This is very very bad:
 1751		  return static_cast<HTMLElement*>(element.get());
That would normally crash.  Grabbing a pointer out of a RefPtr and returning it
is not ok. :(  .release() is what you wanted there.  But again, I don't think
we need this to be a PassRefPtr, or?

Also probably doesn't need to be PassRefPtr:
 1770 PassRefPtr<HTMLOptionElement> HTMLInputElement::selectedOption()

WK style violation:
	 if (element && element->isHTMLElement() &&
element->hasTagName(datalistTag)) {
 1751		  return static_cast<HTMLElement*>(element.get());
 1752	      }