Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[web] Fixes drag scrolling in embedded mode. #53647

Merged
merged 3 commits into from
Jun 29, 2024
Merged

[web] Fixes drag scrolling in embedded mode. #53647

merged 3 commits into from
Jun 29, 2024

Conversation

ditman
Copy link
Member

@ditman ditman commented Jun 29, 2024

When Flutter web runs embedded in a page, scrolling by dragging is interrupted very early by the browser.

It turns out that our pointer events receive a pointercancel + pointerleave by the browser because they happen in an area (the flutter glasspane) that is not really scrollable. See documentation.

Note

After the pointercancel event is fired, the browser will also send pointerout followed by pointerleave.

(Firefox is a good browser to test this, because the browser will cancel our events only if there's scrollable areas around the embedded flutter app.)

There's several solutions, but one of them (used by PixiJS as well) is to cancel the touchstart event that fires with the pointerdown event.

(This PR also removes an unnecessary call to addEventListener in the Listener helper class, and adds some testing to it).

Testing

Issues

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

ditman added 3 commits June 28, 2024 16:03
@ditman
[web] Stop 'touchstart' events.
e4c06c3 
This prevents (mobile) browsers from sending a 'touchcancel' event
shortly after users start to drag on a scrollable list, when the app is
embedded in a host element.

This PR also ensures:

* all pointer events are registered on the _viewTarget. This uses
  `setPointerCapture` so move/up/cancel events keep being reported
  even when the pointer leaves the _viewTarget.
* pointer events are registered only ONCE :)
@ditman
Simplify solution.
47dc0f2
@ditman
Add tests.
729999a
@ditman ditman requested a review from yjbanov June 29, 2024 02:12
@github-actions github-actions bot added the platform-web Code specifically for the web engine label Jun 29, 2024
This was referenced Jun 29, 2024
Closed
Closed
Closed
yjbanov
yjbanov approved these changes Jun 29, 2024
View reviewed changes
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ditman ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 29, 2024
@auto-submit auto-submit bot merged commit f6004b9 into flutter:main Jun 29, 2024
29 checks passed
@ditman ditman deleted the fix-drag-scroll branch June 29, 2024 03:53
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 29, 2024
@engine-flutter-autoroll
f6004b9d6 [web] Fixes drag scrolling in embedded mode. (flutter/engin…
772fc16 
…e#53647)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 29, 2024
@engine-flutter-autoroll
f6004b9d6 [web] Fixes drag scrolling in embedded mode. (flutter/engin…
9ef2b4f 
…e#53647)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 29, 2024
@engine-flutter-autoroll
f6004b9d6 [web] Fixes drag scrolling in embedded mode. (flutter/engin…
99060b0 
…e#53647)
@davidskelly
Copy link

Great job @ditman, thanks for fixing this!

@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 29, 2024
@engine-flutter-autoroll
f6004b9d6 [web] Fixes drag scrolling in embedded mode. (flutter/engin…
b803af7 
…e#53647)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 29, 2024
@engine-flutter-autoroll
f6004b9d6 [web] Fixes drag scrolling in embedded mode. (flutter/engin…
58d3def 
…e#53647)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 29, 2024
@engine-flutter-autoroll
f6004b9d6 [web] Fixes drag scrolling in embedded mode. (flutter/engin…
b7419b7 
…e#53647)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 29, 2024
@engine-flutter-autoroll
f6004b9d6 [web] Fixes drag scrolling in embedded mode. (flutter/engin…
37c2ad6 
…e#53647)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 30, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 30, 2024
@engine-flutter-autoroll
f6004b9d6 [web] Fixes drag scrolling in embedded mode. (flutter/engin…
bb83f74 
…e#53647)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jun 30, 2024
@engine-flutter-autoroll
Roll Flutter Engine from ad1343cfb99e to 0d93da7e2fe1 (9 revisions) (#…
1c9c375 
…151062)

flutter/engine@ad1343c...0d93da7

2024-06-30 [email protected] Roll Fuchsia Linux SDK from zU9wwHk8Oji6pArmw... to DSPuQ9O6WejkIBUUh... (flutter/engine#53657)
2024-06-29 [email protected] Roll Dart SDK from e1e2b16d509b to ab6a48438e07 (1 revision) (flutter/engine#53655)
2024-06-29 [email protected] Roll Skia from 2df28ce437cc to 34be7c830184 (1 revision) (flutter/engine#53654)
2024-06-29 [email protected] Roll Skia from bc0e9542ce83 to 2df28ce437cc (1 revision) (flutter/engine#53652)
2024-06-29 [email protected] Roll Fuchsia Linux SDK from H_P7EHb4zXXV-Eiik... to zU9wwHk8Oji6pArmw... (flutter/engine#53649)
2024-06-29 [email protected] [web] Fixes drag scrolling in embedded mode. (flutter/engine#53647)
2024-06-29 [email protected] Roll Dart SDK from 160ace00da8a to e1e2b16d509b (1 revision) (flutter/engine#53646)
2024-06-28 [email protected] Roll Skia from 1ad84eb8e94f to bc0e9542ce83 (1 revision) (flutter/engine#53644)
2024-06-28 [email protected] Roll Dart SDK from 89b164e3adeb to 160ace00da8a (1 revision) (flutter/engine#53643)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from H_P7EHb4zXXV to DSPuQ9O6Wejk

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request Jul 8, 2024
@engine-flutter-autoroll @victorsanni
Roll Flutter Engine from ad1343cfb99e to 0d93da7e2fe1 (9 revisions) (f…
cbf43d4 
…lutter#151062)

flutter/engine@ad1343c...0d93da7

2024-06-30 [email protected] Roll Fuchsia Linux SDK from zU9wwHk8Oji6pArmw... to DSPuQ9O6WejkIBUUh... (flutter/engine#53657)
2024-06-29 [email protected] Roll Dart SDK from e1e2b16d509b to ab6a48438e07 (1 revision) (flutter/engine#53655)
2024-06-29 [email protected] Roll Skia from 2df28ce437cc to 34be7c830184 (1 revision) (flutter/engine#53654)
2024-06-29 [email protected] Roll Skia from bc0e9542ce83 to 2df28ce437cc (1 revision) (flutter/engine#53652)
2024-06-29 [email protected] Roll Fuchsia Linux SDK from H_P7EHb4zXXV-Eiik... to zU9wwHk8Oji6pArmw... (flutter/engine#53649)
2024-06-29 [email protected] [web] Fixes drag scrolling in embedded mode. (flutter/engine#53647)
2024-06-29 [email protected] Roll Dart SDK from 160ace00da8a to e1e2b16d509b (1 revision) (flutter/engine#53646)
2024-06-28 [email protected] Roll Skia from 1ad84eb8e94f to bc0e9542ce83 (1 revision) (flutter/engine#53644)
2024-06-28 [email protected] Roll Dart SDK from 89b164e3adeb to 160ace00da8a (1 revision) (flutter/engine#53643)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from H_P7EHb4zXXV to DSPuQ9O6Wejk

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@ditman ditman mentioned this pull request Jul 16, 2024
Merged
8 tasks
auto-submit bot pushed a commit that referenced this pull request Jul 17, 2024
@ditman
[web] Set touch-action:none in embedded views. (#53945)
5bf0f03 
This PR adds `touch-action:none` to `flutter-view` elements, so the browser lets the flutter engine fully handle all touch gestures.

This fix is more delicate than the first approach, which broke some merged taps when accessibility/semantics are enabled. 

## Issues

* Found while testing: flutter/flutter#130950
* "More correct" fix for: #53647

## Demos

* Flutter scroll: https://dit-multiview-scroll.web.app
* Semantics: https://dit-tests.web.app
* Scrollable platform views: https://dit-multiview-tests.web.app

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
ditman added a commit to ditman/flutter-engine that referenced this pull request Jul 19, 2024
@ditman
[web] Set touch-action:none in embedded views. (flutter#53945)
c509b0c 
This PR adds `touch-action:none` to `flutter-view` elements, so the browser lets the flutter engine fully handle all touch gestures.

This fix is more delicate than the first approach, which broke some merged taps when accessibility/semantics are enabled. 

## Issues

* Found while testing: flutter/flutter#130950
* "More correct" fix for: flutter#53647

## Demos

* Flutter scroll: https://dit-multiview-scroll.web.app
* Semantics: https://dit-tests.web.app
* Scrollable platform views: https://dit-multiview-tests.web.app

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This was referenced Jul 19, 2024
Merged
Closed
@kevmoo kevmoo added the cp: stable cherry pick to the stable release candidate branch label Aug 8, 2024
@flutteractionsbot
Copy link

Failed to create CP due to merge conflicts.
You will need to create the PR manually. See the cherrypick wiki for more info.

auto-submit bot pushed a commit that referenced this pull request Aug 14, 2024
@ditman
[web] Set touch-action:none in embedded views. (#53945) (#54007)
bb9e2fd 
This PR adds `touch-action:none` to `flutter-view` elements, so the browser lets the flutter engine fully handle all touch gestures.

This fix is more delicate than the first approach, which broke some merged taps when accessibility/semantics are enabled. 

## Issues

* Found while testing: flutter/flutter#130950
* "More correct" fix for: #53647

## Demos

* Flutter scroll: https://dit-multiview-scroll.web.app
* Semantics: https://dit-tests.web.app
* Scrollable platform views: https://dit-multiview-tests.web.app

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

*Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.*

*List which issues are fixed by this PR. You must list at least one issue.*

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

---

### Cherry-pick of #53945

* Fixes: flutter/flutter#152047
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yjbanov yjbanov yjbanov approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App cp: stable cherry pick to the stable release candidate branch platform-web Code specifically for the web engine
Projects
None yet
Milestone
No milestone
5 participants
@ditman @davidskelly @flutteractionsbot @yjbanov @kevmoo