Bug 64937 – Add more text input types for chromium

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Peng Huang]

Currently, webkit only exposes three text input types to chromium (None, Password, Text). We need more types.

[Peng Huang]

Created attachment 101580 [details]
Add more text input types for chromium

[Peng Huang]

Hi Eric, I created a patch for this bug. Could you please review it? Thanks.

[Peng Huang]

Created attachment 101585 [details]
Rebase patch on HEAD

[WebKit Review Bot]

Attachment 101585 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1

Source/WebKit/chromium/ChangeLog:4:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Peng Huang]

Created attachment 101591 [details]
Fix the style issue

[Peng Huang]

Eris is on vocation. Anyone else could review my patch? Thanks.

[Darin Fisher (:fishd, Google)]

Comment on attachment 101591 [details]
Fix the style issue

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

> Source/WebKit/chromium/public/WebTextInputType.h:47
> +    WebTextInputTypeSearch,

These ones seem a bit analogous to WebTextInputTypePassword, but that one
has a rather verbose comment, and these ones do not.  It seems like replicating
that comment for each of these, and just changing password to email, etc., would
be overkill.  Maybe you should re-organize this so that it looks more like so:

  // Input caret is in a specific input field, an input method may be used only if
  // it's suitable for the specific input field.
  WebTextInputTypePassword,
  WebTextInputTypeEmail,
  WebTextInputTypeNumber,
  WebTextInputTypeTelephone,
  WebTextInputTypeURL,

^^^ no need for new lines between them, but you can still separate
WebTextInputTypeNone and WebTextInputTypeText with new lines because
those are conceptually quite different from the group of specific
input types.

> Source/WebKit/chromium/src/WebViewImpl.cpp:1485
> +            if (input.isPasswordField())

maybe this should be a method on WebInputElement?

also, when we are inside the WebKit implementation, we ordinarily just
use WebCore directly instead of creating a WebKit API type in order to
use the WebKit API.  the code is often a bit lighterweight when we use
WebCore directly.  have you considered that?

[Peng Huang]

Created attachment 101911 [details]
Fix review issues

[Peng Huang]

Created attachment 101913 [details]
Fix review issues

[WebKit Review Bot]

Comment on attachment 101913 [details]
Fix review issues

Clearing flags on attachment: 101913

Committed r91892: <http://trac.webkit.org/changeset/91892>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[James Su]

This patch may break the support of elements with contenteditable attribute. Please help check it.

[Peng Huang]

(In reply to comment #12)
> This patch may break the support of elements with contenteditable attribute. Please help check it.

I tested it with contexteditable="true". The input method works fine with it.

[Peng Huang]

Created attachment 102260 [details]
test page

[James Su]

Yes, this patch is ok. Thanks.