Bug 64937 - Add more text input types for chromium
Summary: Add more text input types for chromium
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-21 06:36 PDT by Peng Huang
Modified: 2011-07-28 21:19 PDT (History)
4 users (show)

See Also:


Attachments
Add more text input types for chromium (6.08 KB, patch)
2011-07-21 06:38 PDT, Peng Huang
no flags Details | Formatted Diff | Diff
Rebase patch on HEAD (6.11 KB, patch)
2011-07-21 07:14 PDT, Peng Huang
no flags Details | Formatted Diff | Diff
Fix the style issue (6.12 KB, patch)
2011-07-21 07:56 PDT, Peng Huang
fishd: review-
Details | Formatted Diff | Diff
Fix review issues (4.47 KB, patch)
2011-07-25 14:36 PDT, Peng Huang
no flags Details | Formatted Diff | Diff
Fix review issues (4.47 KB, patch)
2011-07-25 14:39 PDT, Peng Huang
no flags Details | Formatted Diff | Diff
test page (2.80 KB, text/html)
2011-07-28 08:04 PDT, Peng Huang
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Peng Huang 2011-07-21 06:36:41 PDT
Currently, webkit only exposes three text input types to chromium (None, Password, Text). We need more types.
Comment 1 Peng Huang 2011-07-21 06:38:47 PDT
Created attachment 101580 [details]
Add more text input types for chromium
Comment 2 Peng Huang 2011-07-21 07:04:29 PDT
Hi Eric, I created a patch for this bug. Could you please review it? Thanks.
Comment 3 Peng Huang 2011-07-21 07:14:09 PDT
Created attachment 101585 [details]
Rebase patch on HEAD
Comment 4 WebKit Review Bot 2011-07-21 07:15:47 PDT
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.
Comment 5 Peng Huang 2011-07-21 07:56:27 PDT
Created attachment 101591 [details]
Fix the style issue
Comment 6 Peng Huang 2011-07-25 11:45:54 PDT
Eris is on vocation. Anyone else could review my patch? Thanks.
Comment 7 Darin Fisher (:fishd, Google) 2011-07-25 13:57:33 PDT
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?
Comment 8 Peng Huang 2011-07-25 14:36:06 PDT
Created attachment 101911 [details]
Fix review issues
Comment 9 Peng Huang 2011-07-25 14:39:20 PDT
Created attachment 101913 [details]
Fix review issues
Comment 10 WebKit Review Bot 2011-07-27 19:57:34 PDT
Comment on attachment 101913 [details]
Fix review issues

Clearing flags on attachment: 101913

Committed r91892: <http://trac.webkit.org/changeset/91892>
Comment 11 WebKit Review Bot 2011-07-27 19:57:39 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 James Su 2011-07-27 20:31:43 PDT
This patch may break the support of elements with contenteditable attribute. Please help check it.
Comment 13 Peng Huang 2011-07-28 08:02:56 PDT
(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.
Comment 14 Peng Huang 2011-07-28 08:04:09 PDT
Created attachment 102260 [details]
test page
Comment 15 James Su 2011-07-28 21:19:59 PDT
Yes, this patch is ok. Thanks.