950489 - While changing the range of an input type range the page is moving as well

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Steps to reproduce:
 - Open UI tests
 - Go to the 'Keyboard' section
 - Pan a to the bottom until you see an input range
 - Try to change the value of the input range

Expected result:
 - The value of the input range change following the finger

Actual result:
 - The value of the input range change following the finger, and the page moves as well.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Seems like input range has changed from consuming the event to using a flag in bug 849552

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: range.pagepanned.patch

Let's use the flag information.

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: range.pagepanned.patch

Oups. I messed up the indentation of the previous patch.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8347829 [details] [diff] [review]
range.pagepanned.patch

Review of attachment 8347829 [details] [diff] [review]:
-----------------------------------------------------------------

Minusing because I think this is missing a line in the non-APZ case. See below.

::: dom/ipc/TabChild.cpp
@@ +1816,5 @@
>        return;
>      }
>      if (aStatus == nsEventStatus_eConsumeNoDefault ||
> +        nsIPresShell::gPreventMouseEvents ||
> +        aEvent.mFlags.mMultipleActionsPrevented) {

Does this work? In RecvRealTouchEvent the aEvent passed in is cloned to "localEvent" and that is what is sent to DispatchWidgetEvent, but the event passed to UpdateTapState is the original aEvent. Therefore aEvent.mFlags.mMultipleActionsPrevented is not updated; you have to explicitly do:

aEvent.mFlags.mMultipleActionsPrevented = localEvent.mFlags.mMultipleActionsPrevented;

before calling UpdateTapState in RecvRealTouchEvent.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Also for reference, this is the B2G equivalent of https://bugzilla.mozilla.org/show_bug.cgi?id=849961

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: range.pagepanned.patch

Can't we simply pass localEvent since aEvent.Flags is a read only structure ?

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8348690 [details] [diff] [review]
range.pagepanned.patch

Review of attachment 8348690 [details] [diff] [review]:
-----------------------------------------------------------------

That works too. To be honest though I don't know why we need to check gPreventMouseEvents at all; according to http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#7155 we should be able to get that info from the return status. Ah well.

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://hg.mozilla.org/integration/b2g-inbound/rev/1acc5e2112f2

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1acc5e2112f2

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8cc07247bd90

Comment 11

[Jason Smith [:jsmith]]

Note - this might have caused bug 952170.