[webkit-reviews] review denied: [Bug 49291] Add capability for displaying warnings to autofill popup : [Attachment 73784] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Ilya Sherman
<isherman at chromium.org>'s request for review:
Bug 49291: Add capability for displaying warnings to autofill popup
https://bugs.webkit.org/show_bug.cgi?id=49291

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

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=73784&action=review

> WebCore/ChangeLog:3
>  2010-11-12  Ilya Sherman  <isherman at chromium.org>
>  
> +	   Reviewed by NOBODY (OOPS!).

The ChangeLog diff should start at the top of ChangeLog.

> WebCore/ChangeLog:12
> +	   * platform/chromium/PopupMenuChromium.cpp:
> +	   (WebCore::PopupListBox::selectIndex):
> +	   * platform/chromium/PopupMenuChromium.h:

The list lacks PopupListBox::getRowFont().
We had better write what we change for each of files/functions as possible.

> WebKit/chromium/ChangeLog:3
>  2010-11-12  Ilya Sherman  <isherman at chromium.org>
>  
> +	   Reviewed by NOBODY (OOPS!).

The ChangeLog diff doesn't start at the top of ChangeLog.

> WebKit/chromium/src/AutoFillPopupMenuClient.cpp:254
> +    return m_uniqueIDs[index] == -1;

The rule "uniqueID ==-1 -> warning" looks implicit.  Can we introduce a symbol
for this -1?

I also found WebViewClient.h has no explanation of uniqueID.