Bug 103031 – Better type ahead for DateTimeSymbolicFieldElement

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Keishi Hattori]

Better type ahead for DateTimeSymbolicFieldElement.
We want to support cycling and multiple character prefix.

[Keishi Hattori]

Created attachment 175941 [details]
Patch

[Early Warning System Bot]

Comment on attachment 175941 [details]
Patch

Attachment 175941 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14981596

[Early Warning System Bot]

Comment on attachment 175941 [details]
Patch

Attachment 175941 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15003041

[Kent Tamura]

Comment on attachment 175941 [details]
Patch

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

> Source/WebCore/ChangeLog:16
> +        * WebCore.gypi: Added TypeAhead.{h,cpp}

You need to add these files to many other build files.

> Source/WebCore/html/HTMLSelectElement.h:111
> +    // TypeAheadDataSource methods.
> +    virtual int indexOfSelectedOption() const OVERRIDE;
> +    virtual int optionCount() const OVERRIDE;
> +    virtual String optionAtIndex(int index) const OVERRIDE;

Let's make them private because they shouldn't be called by non-TypeAhead code.

> Source/WebCore/html/TypeAhead.cpp:2
> + * Copyright (C) 2012 Google Inc. All rights reserved.

You have to respect the copyright of the original code.

[Kent Tamura]

Comment on attachment 175941 [details]
Patch

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

> Source/WebCore/html/TypeAhead.cpp:51
> +    int length = string.length();
> +
> +    int i;

nit: They should be unsigned.

> Source/WebCore/html/TypeAhead.cpp:79
> +        m_buffer.append(c);

We should avoid String::append.  m_buffer should be a StringBuilder or a Vector.

> Source/WebCore/html/TypeAhead.h:47
> +    int handleEvent(KeyboardEvent*);

You had better add a comment about the return value.

[Keishi Hattori]

Created attachment 175974 [details]
Patch

[Keishi Hattori]

Created attachment 175975 [details]
Patch

[Build Bot]

Comment on attachment 175975 [details]
Patch

Attachment 175975 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14989565

[EFL EWS Bot]

Comment on attachment 175975 [details]
Patch

Attachment 175975 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14987640

[WebKit Review Bot]

Comment on attachment 175975 [details]
Patch

Attachment 175975 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14986586

[Keishi Hattori]

Created attachment 175979 [details]
Patch

[EFL EWS Bot]

Comment on attachment 175979 [details]
Patch

Attachment 175979 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14981673

[Keishi Hattori]

Created attachment 175981 [details]
Patch

[Kent Tamura]

Comment on attachment 175981 [details]
Patch

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

> Source/WebCore/html/TypeAhead.h:59
> +    void setMatchMode(MatchModeFlags);

nit: We can remove setMatchMode and m_matchMode by adding MatchModeFlags argument to handleEvent.
It's up to you to do it.

[Keishi Hattori]

Created attachment 176662 [details]
Patch

[Keishi Hattori]

Created attachment 176665 [details]
Patch

[WebKit Review Bot]

Comment on attachment 176665 [details]
Patch

Clearing flags on attachment: 176665

Committed r136111: <http://trac.webkit.org/changeset/136111>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by bug 103620

[Keishi Hattori]

Created attachment 176699 [details]
Patch

[Keishi Hattori]

Created attachment 176897 [details]
Patch

[WebKit Review Bot]

Comment on attachment 176897 [details]
Patch

Clearing flags on attachment: 176897

Committed r136205: <http://trac.webkit.org/changeset/136205>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[David Kilzer (:ddkilzer)]

<rdar://problem/13334846>