Add @inputmode to input element

RESOLVED FIXED in mozilla17

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: mounir, Assigned: Zoe Bellot)

Tracking

(Blocks: 1 bug, {dev-doc-needed, student-project})

Trunk
mozilla17
dev-doc-needed, student-project
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mentor=mounir][lang=c++])

Attachments

(2 attachments, 4 obsolete attachments)

(Reporter)

Description

5 years ago
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
(Reporter)

Updated

5 years ago
Blocks: 737763
(Assignee)

Comment 1

5 years ago
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.
(Reporter)

Comment 2

5 years ago
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.
(Assignee)

Comment 3

5 years ago
Created attachment 629757 [details] [diff] [review]
add attribute inputmode without keyboard management
Attachment #629757 - Flags: review?(mounir)
(Reporter)

Updated

5 years ago
Assignee: nobody → bellot.zoe
Whiteboard: [mentor=mounir][lang=c++]
(Reporter)

Comment 4

5 years ago
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)
Attachment #629757 - Flags: superreview?(bugs)
Attachment #629757 - Flags: review?(mounir)
Attachment #629757 - Flags: review+

Comment 5

5 years ago
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
Attachment #629757 - Flags: superreview?(bugs) → superreview-

Comment 6

5 years ago
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.
(Assignee)

Comment 7

5 years ago
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?
Attachment #629757 - Attachment is obsolete: true
Attachment #630934 - Flags: review?(mounir)
(Reporter)

Comment 8

5 years ago
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|
Attachment #630934 - Flags: review?(mounir) → feedback+
(Reporter)

Comment 9

5 years ago
(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
(Assignee)

Comment 10

5 years ago
Created attachment 632248 [details] [diff] [review]
Declaration of attribute inputmode
Attachment #630934 - Attachment is obsolete: true
Attachment #632248 - Flags: review?(mounir)
(Assignee)

Comment 11

5 years ago
Created attachment 632251 [details] [diff] [review]
Keyboard management for attribute inputmode
Attachment #632251 - Flags: review?(mounir)
(Reporter)

Comment 12

5 years ago
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.
Attachment #632248 - Flags: superreview?(bugs)
Attachment #632248 - Flags: review?(mounir)
Attachment #632248 - Flags: review+
(Reporter)

Comment 13

5 years ago
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.
Attachment #632251 - Flags: review?(wjohnston)
Attachment #632251 - Flags: review?(mounir)
Attachment #632251 - Flags: feedback+
(Assignee)

Comment 14

5 years ago
Created attachment 632593 [details] [diff] [review]
Declaration of attribute inputmode

declaration with modifications of the review
Attachment #632248 - Attachment is obsolete: true
Attachment #632248 - Flags: superreview?(bugs)
Attachment #632593 - Flags: review?(mounir)
(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.
inputelement.inputmode shouldn't return "". It should have some value by default.
I suggested "auto".
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.
Attachment #632251 - Flags: review?(wjohnston) → review?(cpeterson)
Sorry for the delay. Slipped through my email somehow.
(Reporter)

Updated

5 years ago
Attachment #632593 - Flags: superreview?(bugs)
Attachment #632593 - Flags: review?(mounir)
Attachment #632593 - Flags: review+
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.
Attachment #632251 - Flags: review?(cpeterson) → review+
(Assignee)

Comment 20

5 years ago
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.
Attachment #632251 - Attachment is obsolete: true
Attachment #634335 - Flags: review?(cpeterson)
Comment on attachment 634335 [details] [diff] [review]
Keyboard management for attribute inputmode

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

Thanks, Zoe. LGTM!
Attachment #634335 - Flags: review?(cpeterson) → review+
(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?
(Reporter)

Comment 23

5 years ago
A few searches with "inputmode" and some other keywords (html, javascript) doesn't point to anything that seems relevant.
(Reporter)

Comment 24

5 years ago
Zoe, have those patches been sent to try?
Sent to push a minute ago:
https://tbpl.mozilla.org/?tree=Try&rev=5870ba586f1f

Updated

5 years ago
Attachment #632593 - Flags: superreview?(bugs) → superreview+
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.
This looks like it was forgotten after the try push.
I think zoe the person working on that is busy atm
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
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
(Assignee)

Comment 31

5 years ago
I'm sorry but I haven't time before september or october, so I could modify my patch after september.
(Reporter)

Comment 32

5 years ago
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.
Flags: in-testsuite+
Target Milestone: --- → mozilla17
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 :-)
(Reporter)

Comment 34

5 years ago
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.
https://hg.mozilla.org/mozilla-central/rev/317b3811087c
https://hg.mozilla.org/mozilla-central/rev/b7f9634b073f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Keywords: dev-doc-needed

Updated

5 years ago
Blocks: 783882
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?
> 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?
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
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.
The patch added support for certain values, and after that Hixie added different values to the spec, 
IIRC.
(Reporter)

Updated

5 years ago
Blocks: 809865
(Reporter)

Updated

4 years ago
Depends on: 857355
You need to log in before you can comment on or make changes to this bug.