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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jun 7 10:14:54 PDT 2018


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

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>

More information about the webkit-unassigned mailing list