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

Status: VERIFIED FIXED

(Toolkit :: UI Widgets, defect)

Description

[TheSeer]

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

Comment 1

[TheSeer]

Attachment: Testcase to demo the bizzare behavior

Comment 2

[Ria Klaassen (not reading all bugmail)]

Attachment: reporter's testcase (orginal has small copy error)

Comment 3

[Christophe Charron]

use addEventListner like document.getElementById('tbid').addEventListener('input',functiontocall,false); 
instead of oninput

Comment 4

[David Marteau]

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.

Comment 5

[David Marteau]

Attachment: Test case with use of 'addEventListener' instead of 'oninput'

Comment 6

[TheSeer]

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..

Comment 7

[David Marteau]

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

Comment 9

[Stephanie Daugherty [:sdaugherty]]

platform to all/all and confirming per darktrojan's comments on IRC that this is reproducible on windows.

Comment 10

[[e-mail]]

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

Comment 11

[Geoff Lankow (:darktrojan)]

Attachment: patch

Comment 12

[[e-mail]]

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 13

[[e-mail]]

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.

Comment 14

[Geoff Lankow (:darktrojan)]

Attachment: patch

That also works. Good job I spent more time writing the test than fixing the bug.

Comment 15

[[e-mail]]

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

Since it was my suggestion it's only fair that someone else does the review ;-)

Comment 16

[Neil Deakin]

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)

Comment 17

[Geoff Lankow (:darktrojan)]

Attachment: patch

Of course (spot the Windows developer)

Comment 18

[Neil Deakin]

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

Looks OK. Thanks for the patch.

Comment 19

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/c5aa6fb2f937

Comment 20

[Stephanie Daugherty [:sdaugherty]]

Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:7.0a1) Gecko/20110603 Firefox/7.0a1 ID:20110603030224 verifying fixed