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)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla28
People
(Reporter: jwatt, Assigned: jwatt)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
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 | ||
Comment 1•11 years ago
|
||
Much simpler than the other things we've been discussing. Hopefully this is acceptable.
Attachment #8333629 -
Flags: review?(bugs)
Assignee | ||
Comment 2•11 years ago
|
||
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•11 years ago
|
||
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.
Assignee | ||
Comment 4•11 years ago
|
||
(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.
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #8333629 -
Attachment is obsolete: true
Attachment #8333629 -
Flags: review?(bugs)
Attachment #8333972 -
Flags: review?(bugs)
Comment 6•11 years ago
|
||
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-
Assignee | ||
Comment 7•11 years ago
|
||
Attachment #8333972 -
Attachment is obsolete: true
Attachment #8334046 -
Flags: review?(bugs)
Comment 8•11 years ago
|
||
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+
Assignee | ||
Comment 9•11 years ago
|
||
Comment 10•11 years ago
|
||
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.
Description
•