[webkit-reviews] review denied: [Bug 32533] Add support to set input element's suggestion value from auto-complete menu : [Attachment 45614] PopupMenuClient changes needed for autocomplete suggestions displaying in input elements.

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Zelidrag Hornung
<zelidrag at chromium.org>'s request for review:
Bug 32533: Add support to set input element's suggestion value from
auto-complete menu
https://bugs.webkit.org/show_bug.cgi?id=32533

Attachment 45614: PopupMenuClient changes needed for autocomplete suggestions
displaying in input elements.
https://bugs.webkit.org/attachment.cgi?id=45614&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
Looks fine, except for nits below:

It looks like setInitialAutocompleteValue is only used in one place. Do we need
it as a separate method?

> +void AutocompletePopupMenuClient::setInitialAutocompleteValue()
> +{
> +    if (!m_suggestions.size() || !m_textField->name().length())
> +	   return;
> +    int newIndex = m_selectedIndex >= 0 ? m_selectedIndex : 0;
> +    WebCore::String suggestion = m_suggestions[newIndex];

No need for namespace prefix here.

> +    bool hasPreviousValue =
m_lastFieldValues->contains(m_textField->name());
> +    WebCore::String prevValue;

Ditto.

> +	     if (suggestion.length() > m_typedFieldValue.length()) {
> +		 newSuggestion.append(suggestion.characters() +
m_typedFieldValue.length(),
> +		     suggestion.length() - m_typedFieldValue.length());

Since we're dealing w/UTF16 here, is this still valid for weird characters? I
am a UTF ignoramus, btw.