Closed Bug 927435 Opened 11 years ago Closed 11 years ago

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

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: jwatt, Assigned: jwatt)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
Blocks: 635240
Attached patch value-to-content (obsolete) — Splinter Review
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.
Attached patch patch (obsolete) — Splinter Review
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-
Attached patch patchSplinter Review
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/mozilla-central/rev/764f96661644
Status: NEW → RESOLVED
Closed: 11 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.

Attachment

General

Created:
Updated:
Size: