Last Comment Bug 746142 - Add @inputmode to input element
: Add @inputmode to input element
Status: RESOLVED FIXED
[mentor=mounir][lang=c++]
: dev-doc-needed, student-project
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Zoe Bellot
:
: Andrew Overholt [:overholt]
Mentors:
Depends on: 857355
Blocks: 737763 783882 809865
  Show dependency treegraph
 
Reported: 2012-04-17 07:27 PDT by Mounir Lamouri (:mounir)
Modified: 2013-05-01 09:04 PDT (History)
21 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
add attribute inputmode without keyboard management (5.97 KB, patch)
2012-06-04 06:50 PDT, Zoe Bellot
mounir: review+
bugs: superreview-
Details | Diff | Splinter Review
Declaration of attribute inputmode and management of keyboard (25.67 KB, patch)
2012-06-07 05:54 PDT, Zoe Bellot
mounir: feedback+
Details | Diff | Splinter Review
Declaration of attribute inputmode (7.42 KB, patch)
2012-06-12 07:37 PDT, Zoe Bellot
mounir: review+
Details | Diff | Splinter Review
Keyboard management for attribute inputmode (21.60 KB, patch)
2012-06-12 07:38 PDT, Zoe Bellot
cpeterson: review+
mounir: feedback+
Details | Diff | Splinter Review
Declaration of attribute inputmode (7.42 KB, patch)
2012-06-13 01:08 PDT, Zoe Bellot
mounir: review+
bugs: superreview+
Details | Diff | Splinter Review
Keyboard management for attribute inputmode (21.59 KB, patch)
2012-06-19 03:46 PDT, Zoe Bellot
cpeterson: review+
Details | Diff | Splinter Review

Description Mounir Lamouri (:mounir) 2012-04-17 07:27:43 PDT
The idea behind this attribute would be to give a hint to a IME about what kind of keyboard to show. For example, on mobile, a credit card field shouldn't be <input type=number> because it wouldn't match the widget, the validation and the use case but it should be numeric values only so we need to tell that to the IME. We could also use that for autocapitalized fields or stuff like that.

This attribute was present in WebForms 2 [1].
There are currently some discussions to make them part of the new form features (aka WebForms 3) [2].
Someone wrote a wiki page about some possible use cases [3].
It seems that someone proposed something similar for CSS [4]. Should that be CSS or HTML? I would tend to think it's more appropriate in HTML. IE has a proprietary CSS property similar to that IIRC.

[1] http://www.whatwg.org/specs/web-forms/current-work/#inputmode
[2] https://www.w3.org/Bugs/Public/show_bug.cgi?id=12885
[3] http://wiki.whatwg.org/wiki/Text_input_keyboard_mode_control
[4] http://lists.w3.org/Archives/Public/www-style/2012Feb/0963
Comment 1 Zoe Bellot 2012-06-01 02:12:52 PDT
To declare attribute inputmode, we modified files : nsHTMLInputElement.cpp, nsGKAtomsLIst.h and nsIDOMHTMLInputElement.idl. (inputmode was already declared in nsHTML5AttributeName.cpp and nsHTML5AttributeName.h)
In content/html/content/src/nsHTMLInputElement.cpp : we added declaration of attribute inputmode, the default value and we modified parser to add the value 'numeric'.
In content/base/src/nsGKAtomsList.h and in dom/interface/html/nsIDOMHTMLInputElement.idl: we added attribute inputmode.

To finish, we added a test file (test_bug746142.html in content/html/content/test) to test declaration of attribute inputmode and it seems to work.
Comment 2 Mounir Lamouri (:mounir) 2012-06-01 02:57:13 PDT
It would be better to put the test in content/html/content/test/forms/ and have a clear name like test_inputmode_attribute.html. You could also add some tests in test_input_attributes_reflection.html.

Could you attach the patch here? I think it might be interesting to do that in two parts: first one would be to make the attribute working in tho DOM. The second would be to have in used by the IME.
Comment 3 Zoe Bellot 2012-06-04 06:50:52 PDT
Created attachment 629757 [details] [diff] [review]
add attribute inputmode without keyboard management
Comment 4 Mounir Lamouri (:mounir) 2012-06-04 09:09:21 PDT
Comment on attachment 629757 [details] [diff] [review]
add attribute inputmode without keyboard management

Review of attachment 629757 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, the patch looks good and the test too.

Olli, can you super-review that. In other words, do you think that feature would be useful? (I do, FWIW)
Comment 5 Olli Pettay [:smaug] 2012-06-07 04:22:58 PDT
Comment on attachment 629757 [details] [diff] [review]
add attribute inputmode without keyboard management


>+static const PRUint8 NS_INPUT_INPUTMODE_DEFAULT     = 0;
>+static const PRUint8 NS_INPUT_INPUTMODE_NUMERIC     = 1;
>+
>+static const nsAttrValue::EnumTable kInputInputmodeTable[] = {
>+  {  "", NS_INPUT_INPUTMODE_DEFAULT },
>+  { "numeric", NS_INPUT_INPUTMODE_NUMERIC },
>+  { 0 }
>+};
default value "" is a bit odd, IMO. Perhaps "auto"?


>+++ b/dom/interfaces/html/nsIDOMHTMLInputElement.idl
>@@ -37,16 +37,18 @@ interface nsIDOMHTMLInputElement : nsIDO
uuid should be updated
Comment 6 Olli Pettay [:smaug] 2012-06-07 04:24:06 PDT
Also, since this is adding a new property to input element, could you check whether inputmode is
used commonly in script libraries. We don't want to break sites with this change.
Comment 7 Zoe Bellot 2012-06-07 05:54:56 PDT
Created attachment 630934 [details] [diff] [review]
Declaration of attribute inputmode and management of keyboard

We tested on www.lahab.sandbox.nodester.com and it's working. Next step : add other values for inputmode?
Comment 8 Mounir Lamouri (:mounir) 2012-06-07 07:07:22 PDT
Comment on attachment 630934 [details] [diff] [review]
Declaration of attribute inputmode and management of keyboard

Review of attachment 630934 [details] [diff] [review]:
-----------------------------------------------------------------

In general, that looks good except for the variable naming. I would prefer *ModeHint instead of *InputMode.

Could you attach the patch in two parts: the first one being more or less like the first you attached (with Olli's comments applied) and the second would be the keyboard management. Those two patchs will not be reviewed by the same persons.

::: embedding/android/GeckoSurfaceView.java
@@ +62,5 @@
>          mEditableFactory = Editable.Factory.getInstance();
>          initEditable("");
>          mIMEState = IME_STATE_DISABLED;
>          mIMETypeHint = "";
> +        mIMEInputmode = "";

nit: I would name this mIMEModeHint

@@ +503,5 @@
>          else if (mIMETypeHint.equalsIgnoreCase("time"))
>              outAttrs.inputType = InputType.TYPE_CLASS_DATETIME |
>                                   InputType.TYPE_DATETIME_VARIATION_TIME;
>  
> +        if (mIMEInputmode.equalsIgnoreCase("numeric"))

This should be |else if| because if you have:
<input type='email' inputmode='numeric'>, the user will get a number keyboard but an email one is needed.

::: mobile/android/base/GeckoInputConnection.java
@@ +762,5 @@
>          else if (mIMETypeHint.equalsIgnoreCase("time"))
>              outAttrs.inputType = InputType.TYPE_CLASS_DATETIME |
>                                   InputType.TYPE_DATETIME_VARIATION_TIME;
>  
> +        if (mIMEInputmode.equalsIgnoreCase("numeric"))

|else if|
Comment 9 Mounir Lamouri (:mounir) 2012-06-07 07:36:59 PDT
(In reply to zoeB from comment #7)
> Created attachment 630934 [details] [diff] [review]
> Declaration of attribute inputmode and management of keyboard
> 
> We tested on www.lahab.sandbox.nodester.com and it's working. Next step :
> add other values for inputmode?

On top of my head, I think we should have values like:
- 'numeric': 0-9, +, -, comma, dot;
- 'digit': 0-9 only;
- 'uppercase': A-Z only
- 'lowercase': a-z only
- 'titlecase': uppercase character for each new word [1]
- 'autocapitalized': first letter is uppercased

[1] not sure, this is doable in Android
Comment 10 Zoe Bellot 2012-06-12 07:37:16 PDT
Created attachment 632248 [details] [diff] [review]
Declaration of attribute inputmode
Comment 11 Zoe Bellot 2012-06-12 07:38:38 PDT
Created attachment 632251 [details] [diff] [review]
Keyboard management for attribute inputmode
Comment 12 Mounir Lamouri (:mounir) 2012-06-12 08:03:48 PDT
Comment on attachment 632248 [details] [diff] [review]
Declaration of attribute inputmode

Review of attachment 632248 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the requested changes.

::: content/html/content/src/nsHTMLInputElement.cpp
@@ +145,5 @@
>  
>  // Default autocomplete value is "".
>  static const nsAttrValue::EnumTable* kInputDefaultAutocomplete = &kInputAutocompleteTable[0];
>  
> +static const PRUint8 NS_INPUT_INPUTMODE_DEFAULT           = 0;

Name this NS_INPUT_INPUTMODE_AUTO instead.

::: content/html/content/test/forms/test_input_attributes_reflection.html
@@ +106,5 @@
> +// .inputmode
> +reflectLimitedEnumerated({
> +  element: document.createElement("input"),
> +  attribute: "inputmode",
> +  validValues: [ "numeric", "digit", "uppercase", "lowercase", "titlecase", "autocapitalized" ],

Add 'auto' to the list of valid values.
Comment 13 Mounir Lamouri (:mounir) 2012-06-12 08:12:47 PDT
Comment on attachment 632251 [details] [diff] [review]
Keyboard management for attribute inputmode

Review of attachment 632251 [details] [diff] [review]:
-----------------------------------------------------------------

f+ because I'm not a peer for those modules.

Wes might be able to review part of this.
Comment 14 Zoe Bellot 2012-06-13 01:08:46 PDT
Created attachment 632593 [details] [diff] [review]
Declaration of attribute inputmode

declaration with modifications of the review
Comment 15 Henri Sivonen (:hsivonen) 2012-06-14 02:03:56 PDT
(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #12)
> ::: content/html/content/test/forms/test_input_attributes_reflection.html
> @@ +106,5 @@
> > +// .inputmode
> > +reflectLimitedEnumerated({
> > +  element: document.createElement("input"),
> > +  attribute: "inputmode",
> > +  validValues: [ "numeric", "digit", "uppercase", "lowercase", "titlecase", "autocapitalized" ],
> 
> Add 'auto' to the list of valid values.

Does "auto" mean something different than the absence of the attribute? If it means the same, I'd rather use "" to mean the same as absence than to specify a keyword that means the same as absence.
Comment 16 Olli Pettay [:smaug] 2012-06-14 04:13:10 PDT
inputelement.inputmode shouldn't return "". It should have some value by default.
I suggested "auto".
Comment 17 Wesley Johnston (:wesj) 2012-06-18 10:44:22 PDT
Comment on attachment 632251 [details] [diff] [review]
Keyboard management for attribute inputmode

This looks good to me at a glance, but Chris has to deal with this stuff far more often than me, so I'd rather he approved of changes to it.
Comment 18 Wesley Johnston (:wesj) 2012-06-18 10:44:57 PDT
Sorry for the delay. Slipped through my email somehow.
Comment 19 Chris Peterson [:cpeterson] 2012-06-18 11:31:58 PDT
Comment on attachment 632251 [details] [diff] [review]
Keyboard management for attribute inputmode

Review of attachment 632251 [details] [diff] [review]:
-----------------------------------------------------------------

r+

Some questions below, but otherwise LGTM. Note that different Android IMEs will choose to ignore or handle our InputType hints. In particular, ICS's stock keyboard seems to ignore more InputType hints than Gingerbread's stock keyboard.

::: embedding/android/GeckoSurfaceView.java
@@ +503,5 @@
>          else if (mIMETypeHint.equalsIgnoreCase("time"))
>              outAttrs.inputType = InputType.TYPE_CLASS_DATETIME |
>                                   InputType.TYPE_DATETIME_VARIATION_TIME;
>  
> +        else if (mIMEModeHint.equalsIgnoreCase("numeric"))

1. Do you check mIMEModeHint at the end of this block because any mIMETypeHint should overrule any mIMEModeHint?

2. I think we should remove this extra newline before mIMEModeHint checks. I understand that you are separating the mIMEModeHint checks into a tidy block, but I think it may be confusing.

::: mobile/android/base/GeckoInputConnection.java
@@ +845,5 @@
>              outAttrs.inputType = InputType.TYPE_CLASS_DATETIME |
>                                   InputType.TYPE_DATETIME_VARIATION_TIME;
>  
> +        else if (mIMEModeHint.equalsIgnoreCase("numeric"))
> +            outAttrs.inputType = InputType.TYPE_CLASS_NUMBER |

Same notes as above:

1. Does any mIMETypeHint overrule mIMEModeHint?

2. I think we should remove the newline separating mIMETypeHint and mIMEModeHint checks.
Comment 20 Zoe Bellot 2012-06-19 03:46:14 PDT
Created attachment 634335 [details] [diff] [review]
Keyboard management for attribute inputmode

The same patch but without the extra newline.

For your question, we decided that mIMETypeHint overrules mIMETypeMode to avoid breaking already existing use of inputmode.
Comment 21 Chris Peterson [:cpeterson] 2012-06-19 09:55:20 PDT
Comment on attachment 634335 [details] [diff] [review]
Keyboard management for attribute inputmode

Review of attachment 634335 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, Zoe. LGTM!
Comment 22 Olli Pettay [:smaug] 2012-06-21 06:00:12 PDT
(In reply to Olli Pettay [:smaug] from comment #6)
> Also, since this is adding a new property to input element, could you check
> whether inputmode is
> used commonly in script libraries. We don't want to break sites with this
> change.

Did anyone check if script libraries use inputmode property for something?
Comment 23 Mounir Lamouri (:mounir) 2012-06-21 06:13:03 PDT
A few searches with "inputmode" and some other keywords (html, javascript) doesn't point to anything that seems relevant.
Comment 24 Mounir Lamouri (:mounir) 2012-06-21 10:27:25 PDT
Zoe, have those patches been sent to try?
Comment 25 Charly Molter :lahabana 2012-06-22 05:57:14 PDT
Sent to push a minute ago:
https://tbpl.mozilla.org/?tree=Try&rev=5870ba586f1f
Comment 26 Olli Pettay [:smaug] 2012-06-28 12:35:36 PDT
Make sure to continue discussion about the attribute in #whatwg, and prepare for backing out the
change if some other approach is chosen.
But yeah, I think inputmode is a useful feature.
Comment 27 Josh Matthews [:jdm] (on vacation until Dec 5) 2012-07-15 22:47:54 PDT
This looks like it was forgotten after the try push.
Comment 28 Charly Molter :lahabana 2012-07-16 01:01:10 PDT
I think zoe the person working on that is busy atm
Comment 29 Hixie (not reading bugmail) 2012-07-16 11:56:29 PDT
I'm eager to spec this in the HTML spec. Before we can design something, though, we need to know what the use cases are, what keyboards operating systems support, etc.  I am not well placed to do that research, unfortunately. If you are, it would be fantastic if you could help fill in this wiki page:

   http://wiki.whatwg.org/wiki/Text_input_keyboard_mode_control
Comment 30 Hixie (not reading bugmail) 2012-07-20 14:56:24 PDT
I've updated the spec. <textarea>, <input type=text>, and <input type=search> now support an inputmode="" attribute. The attribute takes a keyword. I've defined keywords for latin-script alphabets (English, most European languages) and for Japanese, since those were the only contexts where anyone helped me find out what was needed. Feel free to mail the WHATWG list if you want more locales supported, I just need to know what's needed, it's easy for me to add more to the spec.

http://www.whatwg.org/specs/web-apps/current-work/#attr-inputmode
Comment 31 Zoe Bellot 2012-07-25 01:04:12 PDT
I'm sorry but I haven't time before september or october, so I could modify my patch after september.
Comment 32 Mounir Lamouri (:mounir) 2012-08-17 03:04:36 PDT
Jush pushed to m-i. We will probably have to prefix that given that what has been spec'd and what has been implemented are slightly different.
Comment 33 Ed Morley [:emorley] 2012-08-17 04:34:15 PDT
Backed out for Android builds failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=506268f7735e

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef98bbc9c8d6

Mounir, the try run in comment 25 is almost 2 months old. Please make sure there is a recent try run (with URL in-bug) before pushing to inbound :-)
Comment 34 Mounir Lamouri (:mounir) 2012-08-17 06:07:12 PDT
I did push that on try and fixed issues but I had a merge conflict before pushing to m-i. I guess I did something wrong when merging.
Comment 36 David Flanagan [:djf] 2012-10-11 16:22:27 PDT
Querying the inputmode property of an input element always seems to return "auto", regardless of the setting of the inputmode attribute.

That seems like it is worse than not supporting it at all, because code that does e.inputmode || e.getAttribute('inputmode') will always get "auto".

Should this bug be reopened, is this a different bug?
Comment 37 Boris Zbarsky [:bz] (still a bit busy) 2012-10-11 16:59:57 PDT
> Querying the inputmode property of an input element always seems to return "auto"

That's quite odd.  Our automated tests and my simple test just now:

  <input inputmode="digit" id="foo">
  <script>
    alert(document.getElementById("foo").inputmode);
  </script>

seem to work fine.  How are you testing?
Comment 38 David Flanagan [:djf] 2012-10-11 23:06:47 PDT
I was doing a test like that in the console.  But I was using inputmode="verbatim".  Your test works for me if I leave it as "digit", but fails when I use verbatim or latin-prose (I didn't try others).

I assume this means that gecko does not support verbatim and other mode modes that are specific to touch-based interfaces and so ignores them.  

My issue is that I want to support values like "verbatim" for the Gaia virtual keyboard.  And because gecko reports "auto", I have found that I need to query with getAttribute() instead of just querying the inputmode property.

I'm honestly not sure if this is a bug or a feature. But it is something I'll have to work around in b2g/chrome/content/forms.js
Comment 39 Boris Zbarsky [:bz] (still a bit busy) 2012-10-11 23:22:06 PDT
Yes, "verbatim" is not a supported value of @inputmode in the checked-in patch, looks like.  I can't tell you why.  In general, the set of inputmode values in the checked-in patch doesn't seem to match the spec very well (e.g. it includes values that are not in the spec).  It's definitely worth filing a bug about matching the spec here.
Comment 40 Olli Pettay [:smaug] 2012-10-12 03:02:52 PDT
The patch added support for certain values, and after that Hixie added different values to the spec, 
IIRC.

Note You need to log in before you can comment on or make changes to this bug.