[Webkit-unassigned] [Bug 186151] [Datalist] Allow TextFieldInputType to show and hide suggestions

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=186151

Wenson Hsieh <wenson_hsieh at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #341977|review?                     |review+
              Flags|                            |

--- Comment #6 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 341977
  --> https://bugs.webkit.org/attachment.cgi?id=341977
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=341977&action=review

> Source/WebCore/html/TextFieldInputType.cpp:802
> +        for (unsigned i = 0; auto option = downcast<HTMLOptionElement>(options->item(i)); ++i) {

Ah, I think in general we prefer auto* instead of auto for pointers like this.

> Source/WebCore/html/TextFieldInputType.cpp:822
> +    m_suggestionPicker = nullptr;

This seems to be the only place we null out m_suggestionPicker, and it looks like the plan is for this to be invoked after an IPC message from the UI process? Should we be clearing this out from more places, e.g. when the element loses focus? Though, it looks like the next patch in the sequence will answer my questions :)

Just keep this in mind!

> Source/WebKit/WebProcess/WebCoreSupport/WebDataListSuggestionPicker.cpp:42
> +    , m_page(page) { }

Nit - in most constructors like this, we'd put the braces on new lines like:

{
}

> Source/WebKit/WebProcess/WebCoreSupport/WebDataListSuggestionPicker.h:47
> +    void didSelectOption(const WTF::String&);

Nit - I don't think the WTF:: in WTF::String should be needed here.

> Source/WebKit/WebProcess/WebCoreSupport/WebDataListSuggestionPicker.h:52
> +    __unused WebCore::DataListSuggestionsClient* m_dataListSuggestionsClient;

I assume these __unused are going to be short-lived? (i.e. the next patch will use m_page and m_dataListSuggestionsClient).

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:3566
> +    m_activeDataListSuggestionPicker = dataListSuggestionPicker;

Also here — make sure we clean up m_activeDataListSuggestionPicker in the various ways that might cause the picker to disappear! (e.g. element is blurred programmatically).

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180607/034e1ad4/attachment.html>