Closed Bug 506069 Opened 11 years ago Closed 9 years ago

textbox with type=number does not properly update value for oninput event

Categories

(Toolkit :: XUL Widgets, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla7

People

(Reporter: TheSeer, Assigned: darktrojan)

References

Details

Attachments

(3 files, 3 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.1) Gecko/20090717 Fedora/3.5.1-1.fc11 Firefox/3.5.1
Build Identifier: Mozilla XULRunner 1.9.1.1 - 20090717030423

Every first change in a textbox does not get the new current value when the oninput event is triggered.


Reproducible: Always

Steps to Reproduce:
1. Open attached testcase
2. Select the content of each textbox
3. change the value to 1 by simply typing 1
Actual Results:  
Both textboxes get updated but when set to type number, the labels updated with oninput differ from the one updated by onkeyup

Expected Results:  
Both labels should be in sync
Attached file Testcase to demo the bizzare behavior (obsolete) —
Flags: wanted1.9.2?
use addEventListner like document.getElementById('tbid').addEventListener('input',functiontocall,false); 
instead of oninput
The problem come from the way that 'oninput' does not follow the rules for
event propagation: it is triggered directly from the underlying input textbox
while the computation of the new numeric value is done when 'input' is captured
along the normal chain of event propagation.
(see http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/numberbox.xml#252)

As stated above, Using addEventListener instead of relying on 'oninput' make things work correctly.
As a sidenote, shouldn't changing the value by use of the spinbuttons also trigger the input event? Right now one has to explicitly register a command event listener or use oncommand..
(In reply to comment #6)
> As a sidenote, shouldn't changing the value by use of the spinbuttons also
> trigger the input event? Right now one has to explicitly register a command
> event listener or use oncommand..

I don't think so, "input" is an event used to intercept what the user type in the textbox. If you just want to listen for change  use the 'change' or 'command' event (that is, spin buttons should trigger change events on the textbox). If you want to react to typing use the 'timeout' attribute.
Flags: wanted1.9.2?
Flags: wanted1.9.2-
Flags: blocking1.9.2-
Attachment #390289 - Attachment is obsolete: true
Blocks: 653637
Duplicate of this bug: 521644
platform to all/all and confirming per darktrojan's comments on IRC that this is reproducible on windows.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86 → All
(In reply to comment #7)
> (In reply to comment #6)
> > As a sidenote, shouldn't changing the value by use of the spinbuttons also
> > trigger the input event? Right now one has to explicitly register a command
> > event listener or use oncommand..
> I don't think so, "input" is an event used to intercept what the user type
> in the textbox. If you just want to listen for change  use the 'change' or
> 'command' event (that is, spin buttons should trigger change events on the
> textbox). If you want to react to typing use the 'timeout' attribute.
Ideally (assuming it works) you would use the "change" event to listen to all changes. The value may not even be legal while the user is inputting (consider the case of typing 10 into a box whose minimum is 2.)
Assignee: nobody → geoff
Attached patch patch (obsolete) — Splinter Review
Attachment #536570 - Flags: review?(neil)
Oh, I see the problem now - the input event is used to set the "dirty" flag, but XBL bubbling event handlers are attached after inline event handlers, so that you don't see the dirty value of the numberbox. It seems to me that the easy way to fix this is to capture the event rather than waiting for it to bubble.
Comment on attachment 536570 [details] [diff] [review]
patch

I don't think this approach is going to work with cut'n'paste, drag'n'drop, etc.
Attachment #536570 - Flags: review?(neil) → review-
Attached patch patch (obsolete) — Splinter Review
That also works. Good job I spent more time writing the test than fixing the bug.
Attachment #536570 - Attachment is obsolete: true
Attachment #536576 - Flags: review?(neil)
Comment on attachment 536576 [details] [diff] [review]
patch

Since it was my suggestion it's only fair that someone else does the review ;-)
Attachment #536576 - Flags: review?(neil) → review?(enndeakin)
Comment on attachment 536576 [details] [diff] [review]
patch

>+  synthesizeKey("A", {ctrlKey: true});

This should be 'accelKey' not 'ctrlKey', as the shortcut modifier is not ctrl on some platforms. (although the test happens to work on those where ctrl+a means goto-start-of-line)
Attached patch patchSplinter Review
Of course (spot the Windows developer)
Attachment #536576 - Attachment is obsolete: true
Attachment #536770 - Flags: review?(enndeakin)
Attachment #536576 - Flags: review?(enndeakin)
Comment on attachment 536770 [details] [diff] [review]
patch

Looks OK. Thanks for the patch.
Attachment #536770 - Flags: review?(enndeakin) → review+
Status: NEW → ASSIGNED
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/c5aa6fb2f937
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Component: XUL → XUL Widgets
Keywords: checkin-needed
Product: Core → Toolkit
QA Contact: xptoolkit.widgets → xul.widgets
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:7.0a1) Gecko/20110603 Firefox/7.0a1 ID:20110603030224 verifying fixed
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.