Implement <input type="tel">

RESOLVED FIXED in mozilla1.9.3a5

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

(Blocks: 3 bugs, {dev-doc-complete, html5})

Trunk
mozilla1.9.3a5
dev-doc-complete, html5
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 11 obsolete attachments)

(Assignee)

Description

8 years ago
The input element telephone state is very similar to search and text states. There is no validation mechanism because telephone numbers are quite different between countries.

Comment 1

8 years ago
For input elements, like the telephone state, where there are no visual changes (unless we come up with something nice), what does actually need to be implemented?
(Assignee)

Comment 2

8 years ago
Make sure the type is recognized and make it works correctly.
There is also "User agents may change the punctuation of values that the user enters." even if I do not really get what that means... I should ask the working group.
In my opinion, we can add a specific UI like the search state may have a magnifying glasses, this one may have a telephone icon. We will see.

Comment 3

8 years ago
(In reply to comment #2)
> Make sure the type is recognized and make it works correctly.
> There is also "User agents may change the punctuation of values that the user
> enters." even if I do not really get what that means... I should ask the
> working group.
> In my opinion, we can add a specific UI like the search state may have a
> magnifying glasses, this one may have a telephone icon. We will see.

I see, and yes, an UI-indication would be good. About the punctuation, I think it is the ways phone numbers can be split up. For example:

"555-55-55-55"
"5555-55-55-55"

I think it depends on if it's a home-number or a cellphone number in those cases. The iPhone splits them up in different ways while being typed in, depending on the length I think. If we want to implement something similar, we could look at that system (or any other product that splits them up this way).

Still, perhaps you should ask the working group to be sure that this is actually the kind of "punctuation" that they are talking about.

(P.S. I am no good with phone numbers, those examples might be very odd, but should give you an idea of what I mean.)
(Assignee)

Comment 4

8 years ago
I do not think it is related to splitting. At least, I hope it isn't because the value will probably be checked by the webpage to make sure it is valid. Depending on countries, phone numbers can have 3-digit blocks or 2-digit blocks or even more complex structure. So, the UA can't add '-' itself.

Anyway, I've just send a message to the WG.
volkmar, are these "Implement" bugs not doing anything with form fill? Are there separate bugs elsewhere to handle smarter auto-completion for these fields?

The Contacts Labs add-on stores contact information like names, emails, telephone numbers, etc. Right now we only autofill email addresses, but telephone, etc would be pretty doable too.
(Assignee)

Comment 6

8 years ago
Actually, email and telephone bugs I've opened are about having these types working as expected by the specs. You may want to open a new bug related to autocompletion with Contacts Labs data depending on each of them.

By the way, you should know there is a bug related to the new autocompletion attribute (bug 557628). You don't need it for the feature you want but you should know autocompletion can be disabled with these new attribute.

If you open new bugs for autocompletion with Contacts Labs data, please add me in CC.

Updated

8 years ago
Summary: Implement <input type="telephone"> → Implement <input type="tel">
(Assignee)

Comment 7

8 years ago
Created attachment 437552 [details] [diff] [review]
Tests v1

Like search type (bug 456229), I don't know how I should test that. I would have too many things to test if I would like to test it correctly.
Olli, if you have any recommendation.
Attachment #437552 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 8

8 years ago
Created attachment 437553 [details] [diff] [review]
Patch v1

This patch is adding the 'tel' state for the input element.
I launched a discussion about the punctuation change on whatwg ML. I do not think it should be done and it looks like I'm not alone so this patch is not doing anything related to that. Anyway, it is optional so...

The style of the telephone input should be done in a follow-up I think.
Attachment #437553 - Flags: review?(Olli.Pettay)
(Assignee)

Updated

8 years ago
Attachment #437553 - Attachment description: Patcd v1 → Patch v1
(Assignee)

Comment 9

8 years ago
By the way, Magne, you were right about "User agents may change the punctuation of values that the user enters."... But I think doing that would be a bad idea.

Comment 10

8 years ago
(In reply to comment #9)
> By the way, Magne, you were right about "User agents may change the punctuation
> of values that the user enters."... But I think doing that would be a bad idea.

I agree. It might create confusion between the user and the serverside validator of a site, so it's better to leave the validation to the server/site itself and let the user correct it accordingly.
Comment on attachment 437553 [details] [diff] [review]
Patch v1

>@@ -1464,17 +1465,18 @@ nsHTMLFormElement::IsDefaultSubmitElemen
> PRBool
> nsHTMLFormElement::HasSingleTextControl() const
> {
>   // Input text controls are always in the elements list.
>   PRUint32 numTextControlsFound = 0;
>   PRUint32 length = mControls->mElements.Length();
>   for (PRUint32 i = 0; i < length && numTextControlsFound < 2; ++i) {
>     PRInt32 type = mControls->mElements[i]->GetType();
>-    if (type == NS_FORM_INPUT_TEXT || type == NS_FORM_INPUT_PASSWORD) {
>+    if (type == NS_FORM_INPUT_TEXT || type == NS_FORM_INPUT_PASSWORD ||
>+        type == NS_FORM_INPUT_TELEPHONE) {

Since this kind of change is pretty common in the patch, could you perhaps make
a helper method.
Something like
PRBool IsSingleLineTextControl(PRint32 aType, PRBool aIncludePassword);
That could probably a (static?) method in nsGenericHTMLFormElement.

or perhaps just
PRBool IsSingleLineTextControl(PRBool aIncludePassword); in nsIFormControl?
or whatever works the best.
Attachment #437553 - Flags: review?(Olli.Pettay) → review-
Comment on attachment 437552 [details] [diff] [review]
Tests v1

You could at least test that modifying type attribute (both content and idl) works well.
Test setting the attribute to some invalid value and then to tel, and perhaps to checkbox and then back to tel...something like that.

And some reftest would be good; you could probably just make sure that <input type="tel"> looks like <input type="text">. And would be great to test even dynamic changes; from checkbox to tel and vise versa.

Updated

8 years ago
Attachment #437552 - Flags: review?(Olli.Pettay) → review-
(Assignee)

Comment 13

8 years ago
For the tests, I think I could add type check in bug 551670 tests as there is a function checking that. Ok for the reftests.

For the helper method, I agree it would be better.
(Assignee)

Updated

8 years ago
Blocks: 558063
(Assignee)

Comment 14

8 years ago
Created attachment 437841 [details] [diff] [review]
Tests v2

reftests have been added and tests from bug 551670 have been changed to check the tel type behavior.
Attachment #437552 - Attachment is obsolete: true
Attachment #437841 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 15

8 years ago
I'm marking this bug as depending on bug 551670 because it is changing its tests so bug 551670 will have to be landed first.
Depends on: 551670
(Assignee)

Comment 16

8 years ago
Created attachment 437859 [details] [diff] [review]
Patch v2
Attachment #437553 - Attachment is obsolete: true
Attachment #437859 - Flags: review?(Olli.Pettay)
(Assignee)

Updated

8 years ago
Blocks: 456229
(Assignee)

Comment 17

8 years ago
Created attachment 437881 [details] [diff] [review]
Tests v2.1

Typo fix
Attachment #437841 - Attachment is obsolete: true
Attachment #437881 - Flags: review?(Olli.Pettay)
Attachment #437841 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 18

8 years ago
Created attachment 437882 [details] [diff] [review]
Patch v2.1

Typo fix
Attachment #437859 - Attachment is obsolete: true
Attachment #437882 - Flags: review?(Olli.Pettay)
Attachment #437859 - Flags: review?(Olli.Pettay)

Updated

8 years ago
Attachment #437881 - Flags: review?(Olli.Pettay) → review+
Comment on attachment 437882 [details] [diff] [review]
Patch v2.1


>+  /**
>+   * Returns true if this is a control which has a text field.
>+   * @param  aExcludePassword  to have NS_FORM_INPUT_PASSWORD returning false.
>+   * @return Wheter this is a single line text control.
>+   */
>+  virtual PRBool IsTextControl(PRBool aExcludePassword) const = 0;
>+
>+  /**
>+   * Returns true if this is a control which has a single line text field.
>+   * @param  aExcludePassword  to have NS_FORM_INPUT_PASSWORD returning false.
>+   * @return Wheter this is a text control.
>+   */
>+  virtual PRBool IsSingleLineTextControl(PRBool aExcludePassword) const = 0;

You got the @return comments wrong way in these two.
And it is whether, not wheter.
>     // If readonly is changed for text and password we need to handle
>     // :read-only / :read-write
>     if (aNotify && aName == nsGkAtoms::readonly &&
>-        (mType == NS_FORM_INPUT_TEXT || mType == NS_FORM_INPUT_PASSWORD)) {
>+        IsSingleLineTextControl(PR_FALSE)) {
Update the comment "if readonly is changed..."

Have you checked that autocomplete works with type="tel"? (or does it even need to?)
Does sessionstore handle type="tel" properly? (or does it even need to?)
Does contextmenu work with type="tel"?

There are things like http://mxr.mozilla.org/mozilla-central/source/embedding/browser/webBrowser/nsDocShellTreeOwner.cpp#1747

What about formfillcontroller? (which does also some of the autocomplete)

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/satchel/src/nsStorageFormHistory.cpp#456

I wonder how to change this all so that when adding the input types, there is
no need to change everything again.
There is also http://mxr.mozilla.org/mozilla-central/source/toolkit/spatial-navigation/SpatialNavigation.js#136
Though I'm not sure if fennec still uses spatialnavigation.

Updated

8 years ago
Attachment #437882 - Flags: review?(Olli.Pettay) → review-
(Assignee)

Comment 22

8 years ago
CC'ing David for a11y.
(Assignee)

Updated

8 years ago
Blocks: 558370
(Assignee)

Comment 23

8 years ago
Created attachment 438120 [details] [diff] [review]
Patch v3

This patch is fixing issues mentioned before. I also checked deeply to find other places where mozIsTextField or IsSingleTextLineControl was needed. It has been fixed everytime except for the password manager (follow-up bug 558370). It is highly probable I've missed some places but the base looks ok.
By the way, session store and autocomplete are working.

Do you want me to add new tests ?

By the way, it looks like this patch creates a bug (can't do any input except in menus). I'm not sure it comes from the patch or something else. I will look deeper at that later.
Attachment #437882 - Attachment is obsolete: true
Attachment #438120 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 24

8 years ago
Created attachment 438350 [details] [diff] [review]
Patch v3.1

Some |type != NS_FORM_INPUT_TEXT| were changed to |IsSingleLineTextControl()|, missing the negation. This is now fixed and fixing the bug I saw.

I think we should remove "Check Spelling" from the context menu for input types different than 'text' and 'search'. I'm going to open a follow-up for that.
Attachment #438120 - Attachment is obsolete: true
Attachment #438350 - Flags: review?(Olli.Pettay)
Attachment #438120 - Flags: review?(Olli.Pettay)
(Assignee)

Updated

8 years ago
Blocks: 558651

Comment 25

8 years ago
(In reply to comment #22)
> CC'ing David for a11y.

It sounds there is nothing to do anything special in a11y support. At least I don't know how to say this input is used to type telephone number to AT via existing APIs. However I should get some feedback from the AT groups if we could do anything.

When patch is ready it's necessary to get Marco to test it.
We could "invent" a new state or the like similar to password edits or the like to explicitly state that this is for telephone numbers. Do these inputs also look different in some way? I am specifically thinking of spin edits, where you can input a value (and no letters), but also have arrows to increase or decrease the value. Screen readers do specify that these are "spin edit controls". So if the styling or layout of telephone inputs is special, we should advocate for a new state or a11y role to make these clearly recognizable also for blind users.
(Assignee)

Comment 27

8 years ago
The tel state will probably look like the text state. Maybe a small icon will be added but nothing important.
I think 'number' is going to be a 'spin edit controls'.

For more information about the tel state style, you should follow bug 558063.
Comment on attachment 438350 [details] [diff] [review]
Patch v3.1


>+#define NS_FORM_BUTTON_BUTTON    1
>+#define NS_FORM_BUTTON_RESET     2
>+#define NS_FORM_BUTTON_SUBMIT    3
>+#define NS_FORM_FIELDSET         4
>+#define NS_FORM_INPUT_BUTTON     5
>+#define NS_FORM_INPUT_CHECKBOX   6
>+#define NS_FORM_INPUT_FILE       7
>+#define NS_FORM_INPUT_HIDDEN     8
>+#define NS_FORM_INPUT_RESET      9
>+#define NS_FORM_INPUT_IMAGE     10
>+#define NS_FORM_INPUT_PASSWORD  11
>+#define NS_FORM_INPUT_RADIO     12
>+#define NS_FORM_INPUT_SUBMIT    13
>+#define NS_FORM_INPUT_TELEPHONE 14
Perhaps this could be actually 
NS_FORM_INPUT_TEL because the type is "tel"
>diff -r 90d264fda571 dom/interfaces/html/nsIDOMNSHTMLInputElement.idl
>--- a/dom/interfaces/html/nsIDOMNSHTMLInputElement.idl	Sat Apr 10 16:00:43 2010 +0200
>+++ b/dom/interfaces/html/nsIDOMNSHTMLInputElement.idl	Sun Apr 11 14:00:20 2010 +0200
>@@ -63,9 +63,15 @@ interface nsIDOMNSHTMLInputElement : nsI
>   void mozGetFileNameArray([optional] out unsigned long aLength,
>                            [array,size_is(aLength), retval] out wstring aFileNames);
>   void mozSetFileNameArray([array,size_is(aLength)] in wstring aFileNames,
>                            in unsigned long aLength);
> 
> 	/* convenience */
>   void                      setSelectionRange(in long selectionStart,
>                                               in long selectionEnd);
>+
>+  /**
>+   * This non-standard method prevents to check types manually to know if the
>+   * element is a text field.
>+   */
>+  boolean mozIsTextField(in boolean aExcludePassword);
> };
You're changing the interface, so please update its iid.

>diff -r 90d264fda571 toolkit/spatial-navigation/SpatialNavigation.js
>--- a/toolkit/spatial-navigation/SpatialNavigation.js	Sat Apr 10 16:00:43 2010 +0200
>+++ b/toolkit/spatial-navigation/SpatialNavigation.js	Sun Apr 11 14:00:20 2010 +0200
>@@ -128,18 +128,19 @@ function _onInputKeyPress (event, callba
> 
>   // check to see if we are in a textarea or text input element, and if so,
>   // ensure that we let the arrow keys work properly.
>   if (target instanceof Ci.nsIDOMHTMLHtmlElement) {
>       _focusNextUsingCmdDispatcher(key, callback);
>       return;
>   }
> 
>-  if ((target instanceof Ci.nsIDOMHTMLInputElement && (target.type == "text" || target.type == "password")) ||
>-      target instanceof Ci.nsIDOMHTMLTextAreaElement ) {
>+  if ((target instanceof Ci.nsIDOMNSHTMLInputElement &&
>+       bestElementToFocus.mozIsTextField(false)) ||
>+      target instanceof Ci.nsIDOMHTMLTextAreaElement) {
Why bestElementToFocus.mozIsTextField(false), why not target.mozIsTextField(false)


browser/ and toolkit/ parts look ok to me, but dao or gavin or someone should review them.
And because this changes API, this need sr. Sicking could probably do that.
IMO having mozIsTextField is ok, and it is useful thing.
(Perhaps even something for HTML5)
Attachment #438350 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Updated

8 years ago
Blocks: 559309
(Assignee)

Comment 29

8 years ago
Created attachment 438976 [details] [diff] [review]
Patch v3.2

r=smaug

This is fixing the issues mentioned in previous comment.

Dao, could you review the toolkit/ and browser/ parts of this bug ?
Jonas, could you super-review the content part ?

Thanks :)
Attachment #438350 - Attachment is obsolete: true
Attachment #438976 - Flags: superreview?(jonas)
Attachment #438976 - Flags: review?(dao)
Comment on attachment 438976 [details] [diff] [review]
Patch v3.2

>diff -r 16e9ee5ae28f browser/base/content/browser.js

>+    if (((el instanceof HTMLInputElement && el.mozIsTextField(true)) ||
>+        type == "hidden" || type == "textarea") ||

The indentation in the second line is off, needs one more space.

>diff -r 16e9ee5ae28f toolkit/components/satchel/src/nsFormFillController.cpp

Somebody else should review this, e.g. dolske.
Attachment #438976 - Flags: review?(dao) → review+
(Assignee)

Comment 31

8 years ago
Comment on attachment 438976 [details] [diff] [review]
Patch v3.2

Thank you for your review Dão. I will fix the indentation later (waiting for other changes).
Asking dolske to review per previous comment.
Attachment #438976 - Flags: review?(dolske)
(Assignee)

Updated

8 years ago
Keywords: dev-doc-needed
Blocks: 559493
Attachment #438976 - Flags: superreview?(jonas) → superreview+
Blocks: 559700

Updated

7 years ago
Blocks: 559746
Blocks: 553097
(Assignee)

Updated

7 years ago
Blocks: 345822
Comment on attachment 438976 [details] [diff] [review]
Patch v3.2

So, a metaissue here is if Form Manager should save the values from the new HTML5 input types in the *current* DB or not... We'll lose the type when storing a value, and can't offer suggestions previously entered for the current input type, but that doesn't seem like a problem. Not saving anything would certainly be less useful.

Would be good to file a followup on having Form Manager remember input types, and use those when determining what suggestions to offer for an input.


>--- a/toolkit/components/satchel/src/nsFormFillController.cpp
...
>-    if (type.LowerCaseEqualsLiteral("text") && !isReadOnly &&
>+    nsCOMPtr<nsIFormControl> formControl = do_QueryInterface(input);
>+    if (formControl && formControl->IsSingleLineTextControl(PR_TRUE) &&
>+        !isReadOnly &&
>         (!autocomplete.LowerCaseEqualsLiteral("off") || isPwmgrInput)) {

Is nsIFormControl a scriptable interface? This code (well, nsStorageFormHistory, but not nsFormFillController) is being ported to JS and will be ready to land soonish -- see bug 439716. It'll need to be able to call this, or else have it's own list of control types matching this.


>--- a/toolkit/components/satchel/src/nsStorageFormHistory.cpp
...
>-      if (!type.LowerCaseEqualsLiteral("text"))
>+      // Save values from input fields which support autocomplete attribute
>+      // and having autocomplete enabled.
>+      // autocomplete is supported for type = 'text', 'search', 'url',
>+      // 'telephone', 'email', 'password', 'datetime', 'date', 'week', 'time',
>+      // 'datetime-local', 'number', 'range' and 'color'.
>+      // See: http://dev.w3.org/html5/spec/forms.html#the-input-element

This comment is a little misleading and long. I'd just nuke the whole thing, or briefly mention "// Check if input is a type supported by Form Manager".

>+      // XXX: this is checking if the input element suports the autocomplete
>+      // attribute but it is not final because most types listed above are not
>+      // implemented.

Nuke this too. No "XXX" comments, please. :)

>+      nsCOMPtr<nsIFormControl> formControl = do_QueryInterface(inputElt);
>+      if (formControl && !formControl->IsSingleLineTextControl(PR_TRUE))
>         continue;

Should this be if (!formControl || !formControl->...) ?

>+      // TODO: autocomplete is not disabled only when equals to 'off',
>+      // bug 557628.

I don't think this is correct. Presumably you're talking about it having the "default" value, but that's handled by checking form.autocomplete earlier in ::Notify.

We could, perhaps, handle the case of having a input with autocomplete=on inside a form with autocomplete=off, though I'm not sure if we want to (seems inefficient). This is fodder for another bug, though.

Finally, does IsSingleLineTextControl() have test coverage somewhere? If not, it would be good to add here (see test_form_submission.html). I don't feel suprt strongly about this, though, we can (and will) add tests when form manager picks up full support for HTML5 inputs.
(Assignee)

Updated

7 years ago
Blocks: 562625
(Assignee)

Comment 33

7 years ago
(In reply to comment #32)
> (From update of attachment 438976 [details] [diff] [review])
> So, a metaissue here is if Form Manager should save the values from the new
> HTML5 input types in the *current* DB or not... We'll lose the type when
> storing a value, and can't offer suggestions previously entered for the current
> input type, but that doesn't seem like a problem. Not saving anything would
> certainly be less useful.
> 
> Would be good to file a followup on having Form Manager remember input types,
> and use those when determining what suggestions to offer for an input.

I've open bug 562625. I think it's better to have the values saved even if the Form Manager doesn't remember the input type for the moment.

> >--- a/toolkit/components/satchel/src/nsFormFillController.cpp
> ...
> >-    if (type.LowerCaseEqualsLiteral("text") && !isReadOnly &&
> >+    nsCOMPtr<nsIFormControl> formControl = do_QueryInterface(input);
> >+    if (formControl && formControl->IsSingleLineTextControl(PR_TRUE) &&
> >+        !isReadOnly &&
> >         (!autocomplete.LowerCaseEqualsLiteral("off") || isPwmgrInput)) {
> 
> Is nsIFormControl a scriptable interface? This code (well,
> nsStorageFormHistory, but not nsFormFillController) is being ported to JS and
> will be ready to land soonish -- see bug 439716. It'll need to be able to call
> this, or else have it's own list of control types matching this.

In JS, you can do the same think by QI to nsIDOMNSHTMLInputElement and call mozIsTextField(true).

> >--- a/toolkit/components/satchel/src/nsStorageFormHistory.cpp
> ...
> >-      if (!type.LowerCaseEqualsLiteral("text"))
> >+      // Save values from input fields which support autocomplete attribute
> >+      // and having autocomplete enabled.
> >+      // autocomplete is supported for type = 'text', 'search', 'url',
> >+      // 'telephone', 'email', 'password', 'datetime', 'date', 'week', 'time',
> >+      // 'datetime-local', 'number', 'range' and 'color'.
> >+      // See: http://dev.w3.org/html5/spec/forms.html#the-input-element
> 
> This comment is a little misleading and long. I'd just nuke the whole thing, or
> briefly mention "// Check if input is a type supported by Form Manager".
> 
> >+      // XXX: this is checking if the input element suports the autocomplete
> >+      // attribute but it is not final because most types listed above are not
> >+      // implemented.
> 
> Nuke this too. No "XXX" comments, please. :)

I will remove the comments.

> >+      nsCOMPtr<nsIFormControl> formControl = do_QueryInterface(inputElt);
> >+      if (formControl && !formControl->IsSingleLineTextControl(PR_TRUE))
> >         continue;
> 
> Should this be if (!formControl || !formControl->...) ?

Indeed. Actually, checking for |formControl| is maybe useless as we are doing a QI from |inputElt| which must be a nsIFormControl.

> >+      // TODO: autocomplete is not disabled only when equals to 'off',
> >+      // bug 557628.
> 
> I don't think this is correct. Presumably you're talking about it having the
> "default" value, but that's handled by checking form.autocomplete earlier in
> ::Notify.
> 
> We could, perhaps, handle the case of having a input with autocomplete=on
> inside a form with autocomplete=off, though I'm not sure if we want to (seems
> inefficient). This is fodder for another bug, though.

Please, see bug 557628 where I've attached a patch trying to fix that. I will remove the comment so we can have this discussion in bug 557628.

> Finally, does IsSingleLineTextControl() have test coverage somewhere? If not,
> it would be good to add here (see test_form_submission.html). I don't feel
> suprt strongly about this, though, we can (and will) add tests when form
> manager picks up full support for HTML5 inputs.

I would say |IsSingleLineTextControl| is partly tested by the tests for 'text' and 'password' types. For the 'tel' type, I don't think I will be able to test _everything_. I will add something in test_form_submission.html for the tel input type.

Thank you for your comments :)
I will update the tests and the patch and re-ask for a review.
(Assignee)

Comment 34

7 years ago
Created attachment 442455 [details] [diff] [review]
Tests v3

test_form_submission.html is now checking for <input type='tel'>. I tried to create some helper functions so I will be able to add 'email', 'url' and 'search' easily.
Attachment #437881 - Attachment is obsolete: true
Attachment #442455 - Flags: review?(dolske)
(Assignee)

Comment 35

7 years ago
Created attachment 442459 [details] [diff] [review]
Patch v3.3

r=smaug,dao sr=jonas

Justin, could you review this updated patch ?
Attachment #438976 - Attachment is obsolete: true
Attachment #442459 - Flags: review?(dolske)
Attachment #438976 - Flags: review?(dolske)
Attachment #442459 - Flags: review?(dolske) → review+
Comment on attachment 442455 [details] [diff] [review]
Tests v3

I'd rather not add <input type="tel"> to all the existing tests, in the interest of keeping each on focused on one specific thing (and to avoid potentially having to change a bunch of existing tests when we do better support for HTML5 form controls). All I was really asking about is if there was test coverage to make sure IsSingleLineTextControl() returns true for type==tel...

Probably easiest to add that to test_bug557620.html as a mozIsTextField test, and not change test_form_submission at all (or keep only the change for <form id="form101">).
Attachment #442455 - Flags: review?(dolske) → review-
(Assignee)

Comment 37

7 years ago
Created attachment 443600 [details] [diff] [review]
Tests v3.1

r=smaug

I've removed the form_submission changes and I've added a test for .mozIsTextField.
The change is trivial, no need to re-review I think.
Attachment #442455 - Attachment is obsolete: true
(Assignee)

Comment 38

7 years ago
Created attachment 443602 [details] [diff] [review]
Patch v3.4

r=smaug,dao,dolske sr=sicking

I'm updating the patch to be sure it will apply correctly on the trunk.
I've launched the patch on the try server 10 hours later and with no errors.
Attachment #442459 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/930c42e06471
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Depends on: 565538
Depends on: 565775

Updated

7 years ago
Blocks: 566348
Documented.
Keywords: dev-doc-needed → dev-doc-complete

Updated

7 years ago
Blocks: 581596
You need to log in before you can comment on or make changes to this bug.