If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in mozilla28

Status

()

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

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

Trunk
mozilla28
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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

Updated

4 years ago
Blocks: 635240
Created attachment 8333629 [details] [diff] [review]
value-to-content

Much simpler than the other things we've been discussing. Hopefully this is acceptable.
Attachment #8333629 - Flags: review?(bugs)
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.
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.
(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.
Created attachment 8333972 [details] [diff] [review]
patch
Attachment #8333629 - Attachment is obsolete: true
Attachment #8333629 - Flags: review?(bugs)
Attachment #8333972 - Flags: review?(bugs)
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 ....
Attachment #8333972 - Flags: review?(bugs) → review-
Created attachment 8334046 [details] [diff] [review]
patch
Attachment #8333972 - Attachment is obsolete: true
Attachment #8334046 - Flags: review?(bugs)
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.
Attachment #8334046 - Flags: review?(bugs) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/764f96661644
https://hg.mozilla.org/mozilla-central/rev/764f96661644
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.