859305 - Stop the browser app from panning the page when the user drags an <input type=range>'s thumb [out-of-process case]

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Jonathan Watt [:jwatt]]

When the user tries to drag an <input type=range>'s thumb in the browser app, the page currently scrolls too. We need to stop it doing that.

I thought I fixed this issue in bug 854133, but it seems that that only fixed the browser app when it is running pages in-process. Turns out the in-process case is legacy code that only gets run when the app is occasionally switched to run in-process to make it usable while bad out-of-process bugs are being fixed. Normally the browser app runs out-of-process now. I just got unlucky and happened to debug and "fix" the issue while it was temporarily switched to in-process.

So this bug is about fixing the issue for the normal out-of-process case.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

This patch probably needs a fair bit of background, so here goes:

When the content document runs out-of-process, ideally touch events would be passed from the main process to the content subprocess, the main process would wait until the content process has processed the event, marking it as having been handled or not, and then the main process would handle it if the content process didn't, or ignore it if it did. Of course this conflicts somewhat with the desire to make panning of a page snappy, since it can take hundreds of milliseconds to round-trip the event from the main process to the content process and back again.

When it can, B2G tries to keep panning etc. very responsive by checking whether the content process claims to contain anything that might handle the touch events. If it does not, the main process handles the events itself without waiting for the roundtrip to complete. (So any attempt by the content process to say "this was handled" will have no affect if the main process doesn't know the content process might be handling touch events.)

The thing that currently counts when it comes to deciding whether the content process contains anything that might handle touch events is whether the document has added event listeners for any touch events.

Right now, when TabChild::SendContentReceivedTouch is called to tell the main process that the event has been processed by the content process, it simply passes back gPreventMouseEvents as the aPreventDefault argument that TabParent::RecvContentReceivedTouch will receive (this logic was added in bug 775447):

http://mxr.mozilla.org/mozilla-central/source/dom/ipc/TabChild.cpp?rev=5e01908df6d7&mark=1745#1734

I'm not sure why we're not checking the state of localEvent rather than gPreventMouseEvents, but I guess in practice it's pretty much the same thing since gPreventMouseEvents is set to true whenever the status at the end of a PresShell::DispatchTouchEvent call is nsEventStatus_eConsumeNoDefault.

So, what we really need to do here is two things...

First, we need to get TabChild::SendContentReceivedTouch to send back the value true as the aPreventDefault argument if the <input type=range> has handled the touch event. That's what the layout/base/nsPresShell.cpp change is for, since it seems best to have gPreventMouseEvents set to true in the case the event is handled:

http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp?rev=5e01908df6d7&mark=6963,6979#6962

Additionally, in order to get B2G to wait for the events to round-trip, we need mFrameMetrics.mMayHaveTouchListeners to be true so that we end up calling SetState(WAITING_LISTENERS) here:

http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/AsyncPanZoomController.cpp?rev=5e01908df6d7&mark=293,297,301,304#286

That's what the nsRangeFrame.cpp change is for. (Possibly I should do some renaming of the "MayHaveTouchListeners" variable and functions given this change.)

Comment 2

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

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

>+      nsCOMPtr<nsPIDOMWindow> innerWin(document->GetInnerWindow());
Why nsCOMPtr and not just nsPIDOMWindow?

Comment 3

[Jonathan Watt [:jwatt]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e1336dc1f555

(In reply to Olli Pettay [:smaug] from comment #2)
> Why nsCOMPtr and not just nsPIDOMWindow?

I changed it to a raw pointer.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e1336dc1f555