935501 - Get mouse events working for <input type=number>'s spin up/down buttons

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Jonathan Watt [:jwatt]]

We need <input type=number>'s spin up/down buttons to do the right thing when clicked on. This will be a little less than straightforward if we want the number to keep spinning if the user keeps a button depressed.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Since you're looking at 935506 now...

The idea here is that mousing down on a spin button will immediately step up/down then, after half a second delay, will step again every 50 ms. I'm being pretty aggressive about stopping the timer. That said I do allow the mouse pointer to move from the up to the down button and visa versa while the mouse button is still depressed and it just changes the direction of spin without cancelling the spin. If the mouse leaves a button to a non-button I stop the repeat timer though.

Comment 2

[Jonathan Watt [:jwatt]]

Attachment: patch

Per-IRC, here's a version that uses mouse capture instead of NS_MOUSE_EXIT_SYNTH.

Comment 3

[Jonathan Watt [:jwatt]]

Attachment: patch

I've slightly changed how the code determines whether a pointer event is over one of the spin buttons to better fit with what I'm having to do to get native theming working.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

this is about mouse events, not pointer events, right?

Comment 5

[Jonathan Watt [:jwatt]]

yes

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

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

I think this should use nsRepeatService so that we get consistent behavior with
scrollbars.


>+  if (mType == NS_FORM_INPUT_NUMBER &&
>+      mNumberControlSpinButtonTimer) {
>+    // If the timer is running the user has depressed the mouse on one of the
>+    // spin buttons. If the mouse exits the button we either want to reverse
>+    // the direction of spin if it has moved over the other button, or else we
>+    // want to end the spin. We do this here (rather than in PostHandleEvent)
>+    // because we don't want to let content preventDefault() the ending of the
>+    // spin.
s/the ending/to end/ or something?


>+    if (aVisitor.mEvent->message == NS_MOUSE_MOVE) {
>+      // Be agressive about stoping the timer:
aggressive 
stopping, I think


I'd like to see a new patch using repeat service.

Comment 7

[Jonathan Watt [:jwatt]]

Attachment: patch

(In reply to Olli Pettay [:smaug] from comment #6)
> I think this should use nsRepeatService

Ah, nice. Agreed!

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

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


Make sure the timer is stopped if nsHTMLInputElement dies while the time is active. Otherwise you have a sg:crit bug.

>+  if (mType == NS_FORM_INPUT_NUMBER) {
>+    if (mNumberControlSpinnerIsSpinning) {
>+      // If the timer is running the user has depressed the mouse on one of the
>+      // spin buttons. If the mouse exits the button we either want to reverse
>+      // the direction of spin if it has moved over the other button, or else
>+      // we want to end the spin. We do this here (rather than in
>+      // PostHandleEvent) because we don't want to let content preventDefault()
>+      // the end of the spin.
>+      if (aVisitor.mEvent->message == NS_MOUSE_MOVE) {
trusted event check here please.


>+            if (aVisitor.mEventStatus != nsEventStatus_eConsumeNoDefault) {
>+              // We didn't handle this to step up/down. Whateven this was, be
Whatever
>+              // aggressive about stopping the spin. (And don't set
>+              // nsEventStatus_eConsumeNoDefault after doing so, since that
>+              // might prevent, say, the context menu from opening.)
>+              StopNumberControlSpinnerSpin();
But this all is odd anyway. Content may dispatch mouseup/down to stop spinning?
(this if check doesn't have isTrusted check or anything)

Comment 9

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

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

So we need still some tests for this.
Shouldn't be too hard to write.

Comment 11

[Jonathan Watt [:jwatt]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7fe4999863b7

I added some tests, although to test the spin we need to change the firing of 'input' events as discussed on IRC. I'll file a bug for that shortly and comment with the bug number here.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7fe4999863b7

Comment 13

[Petruta Horea [:phorea], Desktop Test Engineering]

Verified as fixed using the latest Aurora build 29.0a2 (20140304004003) under Win 7 64-bit, Ubuntu 32-bit and Mac OSX 10.9.