927435 - When a user types into <input type=number>'s anonymous text control, update the value of the <input type=number> as appropriate

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jonathan Watt [:jwatt]]

When a user types into <input type=number>'s anonymous text control, we need to update the value of the <input type=number> as appropriate.

We'll somehow need to do this without the update resulting in extra 'input' or 'change' events being dispatched (i.e. events to the anonymous text control causing an update to the number control, resulting in extra events to the number control) since that would be visible to content.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: value-to-content

Much simpler than the other things we've been discussing. Hopefully this is acceptable.

Comment 2

[Jonathan Watt [:jwatt]]

I left in the commented code in HTMLInputElement::PreHandleEvent since I'm not sure if I should be doing something like that in this case.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Could you explain why we need to handle number differently from type="text".

And also, this needs tests. And you probably want to handle only trusted event and possible check
that originalTarget is the native anonymous element.

Comment 4

[Jonathan Watt [:jwatt]]

(In reply to Olli Pettay [:smaug] from comment #3)
> Could you explain why we need to handle number differently from type="text".

I presume you're talking about the different conditions for firing the 'change' event here: If someone focuses a type=text and types "abc", then focuses something else, we should fire a change event because its internal value changes from "" to "abc". If someone does the same for type=number we should _not_ fire a change event because "abc" is not a valid number, and therefore its internal value stays as "".

> And also, this needs tests.

There are lots of tests that fail without this patch. Unfortunately the preexisting tests are making it hard to land the <input type=number> implementation incrementally.

> And you probably want to handle only trusted
> event and possible check
> that originalTarget is the native anonymous element.

Okay.

Comment 5

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8333972 [details] [diff] [review]
patch

>+        aVisitor.mItemFlags |= NS_NO_CONTENT_DISPATCH;
>+        aVisitor.mEvent->mFlags.mNoContentDispatch = true;
These don't look right. mNoContentDispatch should be removed.


>+        //if (aVisitor.mDOMEvent) {
>+        //  aVisitor.mDOMEvent->StopPropagation();
>+        //} else {
Yeah, we don't want to stop propagation.
If you don't want the content DOM to see the event, set aVisitor.mCanHandle to false.


But still trying to understand this all ....

Comment 7

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8334046 [details] [diff] [review]
patch


>+      if (aVisitor.mEvent->message == NS_FORM_CHANGE) {
else if


>+    number.focus();
>+    synthesizeKey("1", {});
>+    is(numberChange, 0, "Change event shouldn't be dispatched on number input element for keyboard input until it is looses focus");
r+ assuming other browsers don't fire change in this case either.

Comment 9

[Jonathan Watt [:jwatt]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/764f96661644

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/764f96661644