Repository navigation
Touchpad scroll on chromebooks doesn't work on stable (1.7.8+hotfix.2) #36118
Description
Summary
Two finger trackpad scroll used to work, and now it doesn't. I did a (sloooow) binary search through the commits between stable releases. The commit which broke trackpad scrolling is 7f969d0. Unfortunately, that was a pretty complex PR (well, at least for my flutter skills), and I wasn't able to figure out what exactly went wrong.
Steps to Reproduce:
I used a slightly modified version of the counter example to make a scrollable list.
import 'package:flutter/material.dart';
void main() => runApp(MyApp());
class MyApp extends StatelessWidget {
@override
Widget build(BuildContext context) {
return MaterialApp(
title: 'Flutter Demo',
theme: ThemeData(
primarySwatch: Colors.blue,
),
home: MyHomePage(title: 'Flutter Demo Home Page'),
);
}
}
class MyHomePage extends StatefulWidget {
MyHomePage({Key key, this.title}) : super(key: key);
final String title;
@override
_MyHomePageState createState() => _MyHomePageState();
}
class _MyHomePageState extends State<MyHomePage> {
int _counter = 0;
void _incrementCounter() {
setState(() {
_counter++;
});
}
@override
Widget build(BuildContext context) {
return Scaffold(
appBar: AppBar(
title: Text(widget.title),
),
body: Center(
child: ListView(
children: [
Text(
'You have pushed the button this many times:',
),
Text(
'$_counter',
style: Theme.of(context).textTheme.display1,
),
SizedBox(height: 16),
...List.generate(_counter, (index) {
return Padding(
padding: EdgeInsets.symmetric(vertical: 8),
child: Text('#$index'),
);
}),
],
),
),
floatingActionButton: FloatingActionButton(
onPressed: _incrementCounter,
tooltip: 'Increment',
child: Icon(Icons.add),
), // This trailing comma makes auto-formatting nicer for build methods.
);
}
}
- Run the app on a chromebook. I tested on an ASUS C202, a Lenovo e300, and
one more (I don't have the model at hand, I'll add it when I can look)an HP Chromebook x360 - Click the
+button until you have a list that goes past the bottom of the window - Try to scroll with two fingers on the trackpad
Expected Result
The app scrolls
Actual Result
The app doesn't scroll
Logs
I didn't see anything in the verbose logs
Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel unknown, v1.6.1-pre.56, on Mac OS X 10.14.5 18F132, locale en-US)
[✓] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
[!] iOS toolchain - develop for iOS devices (Xcode 10.2.1)
✗ Verify that all connected devices have been paired with this computer in Xcode.
If all devices have been paired, libimobiledevice and ideviceinstaller may require updating.
To update with Brew, run:
brew update
brew uninstall --ignore-dependencies libimobiledevice
brew uninstall --ignore-dependencies usbmuxd
brew install --HEAD usbmuxd
brew unlink usbmuxd
brew link usbmuxd
brew install --HEAD libimobiledevice
brew install ideviceinstaller
[✓] Android Studio (version 3.4)
[✓] VS Code (version 1.36.1)
[!] Connected device
Activity
dkwingsmt commentedon Jul 13, 2019
Thank you very much for the minimal example. I'll take a look next Monday.
Skylled commentedon Jul 16, 2019
Running into this issue as well. Verified that plugging in a physical mouse and using the mouse wheel still scrolls the list as expected.
dkwingsmt commentedon Jul 16, 2019
Initial investigation: two-finger scrolling on a trackpad triggers scrolling by sending
PointerMoveEventwithbuttons: 0, which becomes rejected due to this PR.We can either make an exception that allows move events to have
buttons: 0, or see if we can send them using other routes, such as reinterpret them as scroll events.dkwingsmt commentedon Jul 16, 2019
+cc @stuartmorgan I think you implemented trackpad scrolling right? Is it possible or desirable to reinterpret them as scroll events?
(Another choice is to add
button: 1to move events sent by trackpad scrolling)stuartmorgan-g commentedon Jul 16, 2019
I'd have to dig more, but that doesn't sound right. I believe I designed it to use hover events for non-gesture pointer events, with the intention of using down/move/up for gestural pointer events (pinch-zoom, improved trackpad scroll handling, etc.)
We should allow pointer signals to have
buttons: 0dkwingsmt commentedon Jul 16, 2019
@stuartmorgan
PointerSignalEventis allowed to havebuttons: 0, butPointerMoveEventis not.This is how touchpad scrolling works before the PR:
PointerMoveEventis constructed withbuttons: 0and varyingpositionGestureDetectorandDragGestureRecognizerDragGestureRecognizer.onUpdate, which updatesScrollable.Also it seems to me that
PointerMoveEvents are received without the leadingPointerDownEvent.stuartmorgan-g commentedon Jul 16, 2019
I'm confused; the initial description here is talking about two-finger scrolling, which I would expect to be using the pointer signal codepath (Edit: specifically, I would have thought that flutter-team-archive/engine#8149 would be the codepath used for two-finger scrolling of an Anrdoid app on ChromeOS). That wouldn't use gesture recognizers.
Could you link to the specific code that's creating the events you are talking about?
dkwingsmt commentedon Jul 17, 2019
@stuartmorgan I'm still investigating, but here is what I observed: I added the following printing to
PointerEventConverter.expandand when I do a two-finger scrolling, this is what is logged:
And these events used to be the ones that triggered the change of
Scrollable.position. Now they're blocked because they havebuttons: 0.stuartmorgan-g commentedon Jul 17, 2019
I don't have experience with ARC, but it sounds like this is https://developer.android.com/topic/arc/input-compatibility which seems like something we should be disabling for Flutter given that we have scroll handling in the Android code.
Skylled commentedon Jul 17, 2019
Please correct me if I'm wrong, but it might be wiser for Flutter to respect ARC's input, at least for scrolling, as Chrome OS has a setting that allows you to flip the touchpad scrolling direction.
https://support.google.com/chromebook/answer/1047367#settings
If Flutter worked with the raw touchpad input, this setting would be ignored, which would be unexpected behavior for users and a regression from previous Flutter versions. I just verified with an older build of my app that touchpad scrolling flips as expected when that setting is toggled.
stuartmorgan-g commentedon Jul 17, 2019
That is not the suggestion. Android is capable of receiving and handling scroll events, and the PR I linked above uses those to generate pointer signal events; the preferred outcome if it's possible (which I think is likely) is for ChromeOS to deliver two-finger scrolls as scroll events, rather than a series of synthetic touch-and-drag events.
In that model I would imagine that ChromeOS would apply directional setting preferences prior to delivering them as scroll events. (As does, for instance, macOS, which also has such a setting.)
dkwingsmt commentedon Jul 17, 2019
@Skylled brought about a very good point. I flipped the option for scrolling direction, and scrolled on trackpad in the same direction as before (downwards), and got the log as follows:
It now reports
PointerMoveEventwith decreasing Y, instead of increasing Y.stuartmorgan-g commentedon Jul 17, 2019
Again, there's no reason to believe the same reversal would not happen if we can switch to a mode where two-finger scroll is delivered as scroll events rather than synthesized touches.
stuartmorgan-g commentedon Jul 17, 2019
Looking at the ARC page again, I didn't read far enough down:
https://developer.android.com/topic/arc/input-compatibility#mouse_and_touchpad
Since scroll wheels work as expected, it sounds like we aren't in compatibility mode as I thought from the initial description. And it sounds like there's no way to have the OS deliver these the same way as scroll wheel events, so we'd need to do the transformation manually.
Based on the description in that section, my recommendation would be to update the Android embedding to detect two-finger scroll (using
getSourceorgetButtonStateper the docs) and send them as discrete pointer signal events (using computed deltas) instead of passing the synthetic touches through as is currently done. You can leave a TODO to update to gestural scrolling (which that input stream will be a better match for) once #23604 is implemented.dkwingsmt commentedon Jul 19, 2019
@stuartmorgan I tried to implement the feature that converts touchpad scrolling to scroll events. Despite other difficulties, I found out that one of the differences of mouse scrolling and touch scrolling is that
Trackpad scrolling actually follows the latter form in native apps. Therefore it should take the route of touch scrolling instead of mouse scrolling.
stuartmorgan-g commentedon Jul 19, 2019
Yes, and gestural signal scrolling will do that (and did, in fact, in my early implementation). Thus the suggestion of a TODO to upgrade the Android implementation from discrete signal scrolling to gestural signal scrolling later.
Hacking the framework's handling of buttons just to avoid that temporary, minor difference when running Android apps on ChromeOS seems like a poor tradeoff to me, since it'll be harder to undo the button changes later (since other things may start depending on that without anyone realizing it).
If you really feel that it's a major issue, then you could implement signal gestures (#23604) first, and then use that here now, but implementing discrete scrolling first would give most of the functionality back quickly.
stuartmorgan-g commentedon Jul 19, 2019
I forgot about the other route you suggested: adding a primary button to the synthetic events. That would be a simple but localized temporary fix that doesn't have the concern of affecting framework code. Doing that as the short-term fix instead of using discrete pointer signals SGTM.
(The main drawback is that those events could potentially trigger gesture detectors other than scroll, which is not ideal, but apparently that's the way it worked before your button validation change so it would be no worse than it was.)
dkwingsmt commentedon Jul 19, 2019
@stuartmorgan What do you think if
Scrollables use a special private version ofDragGestureRecognizerthat also allows buttonless-mouse events, as a short-term fix?stuartmorgan-g commentedon Jul 19, 2019
Personally I think localizing any workaround to the Android embedding is far preferable to doing anything in the framework to handle this one unusual event source. Any form of the latter is going to be harder to unwind because it'll be very difficult to know if anything else has started relying on it.
wreppun commentedon Aug 6, 2019
@dkwingsmt, thanks for the work on this!
Just curious -- is the plan to wait for #23604 to land? I'm trying to get a release ready for a few chromebook schools by September -- any thoughts on how I could get a short term fix implemented?
stuartmorgan-g commentedon Aug 6, 2019
Given that there are reasonable short-term fix options, I don't think there's a need to wait for 23604.
github-actions commentedon Aug 28, 2021
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of
flutter doctor -vand a minimal reproduction of the issue.