[webkit-reviews] review denied: [Bug 23465] Refactor some more HTMLOptionElement code : [Attachment 26910] Initial patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 23465: Refactor some more HTMLOptionElement code
https://bugs.webkit.org/show_bug.cgi?id=23465

Attachment 26910: Initial patch
https://bugs.webkit.org/attachment.cgi?id=26910&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Seems like this one:
 982	     if
(stripLeadingWhiteSpace(static_cast<HTMLOptionElement*>(items[index])->textInde
ntedToRespectGroupLabel()).startsWith(prefix, false)) {


Doesn't want that call.  Also, that line could be broken into multiple lines to
be more readable.

What is "aggregateOptionText"?	Maybe you mean aggregate as a verb?  Still not
sure why it's used that way.

maybe m_optionData?  m_data is so bland, reminds me of the kde "d" pointers
which I was never a fan of.

I think "aggregate*" I would just call "optionText" and "optionValue".	The
fact that they have to do work under the covers is OK to hide in this case I
think.

Otherwise this looks fine to me.  I'd like to see the final patch though.