Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[cp:beta][web] Work around wrong pointerId in coalesced events in iOS Safari 18.2 (#56719)#56905

Merged
auto-submit[bot] merged 2 commits intoflutter:flutter-3.27-candidate.0from
mdebbar:cp_ios_18_2_beta
Dec 10, 2024
Merged

[cp:beta][web] Work around wrong pointerId in coalesced events in iOS Safari 18.2 (#56719)#56905
auto-submit[bot] merged 2 commits intoflutter:flutter-3.27-candidate.0from
mdebbar:cp_ios_18_2_beta

Conversation

@mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Dec 2, 2024 •
edited
Loading

Manual cherry pick for #56719

Cherrypick request: flutter/flutter#159692

@mdebbar
[cp:beta][web] Work around wrong pointerId in coalesced events in iOS…
c307079 
… Safari 18.2 (flutter#56719)

In iOS 18.2, Safari [added support](https://developer.apple.com/documentation/safari-release-notes/safari-18_2-release-notes#Web-API) for the [`getCoalescedEvents`](https://developer.mozilla.org/en-US/docs/Web/API/PointerEvent/getCoalescedEvents) API.

That being said, the API seems to be incomplete (or at least doesn't match other browsers' behavior). The coalesced events lack a [`pointerId`](https://developer.mozilla.org/en-US/docs/Web/API/PointerEvent/pointerId) and [`target`](https://developer.mozilla.org/en-US/docs/Web/API/Event/target) properties.

I'm not sure if this issue will be fixed in the stable release of iOS 18.2, so in the meantime, this PR implements a workaround to avoid this issue.

Fixes flutter/flutter#158299
Fixes flutter/flutter#155987
@github-actions github-actions bot added the platform-web Code specifically for the web engine label Dec 2, 2024
@mdebbar mdebbar mentioned this pull request Dec 2, 2024
Closed
ditman
ditman approved these changes Dec 2, 2024
View reviewed changes
Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for cherry-picking!

yjbanov
yjbanov approved these changes Dec 2, 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

@givip
Copy link

givip commented Dec 6, 2024

Is there any chance to get these changes merged soon?

@christopherfujino
Copy link
Contributor

christopherfujino commented Dec 6, 2024

@mdebbar there are failing tests in Mac mac_unopt:

 [3074/3074] SaturatedMath.CastingFiniteDoubleToFloatStaysFinite (59 ms)
 FAILED TESTS (9/3074):
      589 ms: /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/AiksTest.GaussianBlurSolidColorTinyMipMap/Metal (try #1)
      497 ms: /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/GaussianBlurFilterContentsTest.RenderCoverageMatchesGetCoverageTranslate/Metal (try #1)
      552 ms: /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/GaussianBlurFilterContentsTest.TextureContentsWithEffectTransform/Metal (try #1)
      447 ms: /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/GaussianBlurFilterContentsTest.TextureContentsWithDestinationRectScaled/Metal (try #1)
      582 ms: /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/AiksTest.CanRenderInvertedImageWithColorFilter/Metal (try #2)
      568 ms: /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/GaussianBlurFilterContentsTest.RenderCoverageMatchesGetCoverage/Metal (try #2)
      422 ms: /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/MatrixFilterContentsTest.RenderCoverageMatchesGetCoverageScale/Metal (try #2)
      431 ms: /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/GaussianBlurFilterContentsTest.RenderCoverageMatchesGetCoverageRotated/Metal (try #1)
      524 ms: /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/EntityTest.RuntimeEffectCanPrecache/Metal (try #1)
 SUMMARY:
   /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/AiksTest.CanRenderInvertedImageWithColorFilter/Metal passed 1 / 2 times.
   /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/AiksTest.GaussianBlurSolidColorTinyMipMap/Metal passed 1 / 2 times.
   /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/EntityTest.RuntimeEffectCanPrecache/Metal passed 1 / 2 times.
   /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/GaussianBlurFilterContentsTest.RenderCoverageMatchesGetCoverage/Metal passed 1 / 2 times.
   /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/GaussianBlurFilterContentsTest.RenderCoverageMatchesGetCoverageRotated/Metal passed 1 / 2 times.
   /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/GaussianBlurFilterContentsTest.RenderCoverageMatchesGetCoverageTranslate/Metal passed 1 / 2 times.
   /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/GaussianBlurFilterContentsTest.TextureContentsWithDestinationRectScaled/Metal passed 1 / 2 times.
   /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/GaussianBlurFilterContentsTest.TextureContentsWithEffectTransform/Metal passed 1 / 2 times.
   /Volumes/Work/s/w/ir/cache/builder/src/out/ci/host_debug_tests/impeller_unittests Play/MatrixFilterContentsTest.RenderCoverageMatchesGetCoverageScale/Metal passed 1 / 2 times.

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8729553482934559489/+/u/test:_Host_Tests_for_host_debug/stdout

@mdebbar
Copy link
Contributor Author

mdebbar commented Dec 9, 2024

@christopherfujino I'm not sure how my web-only change could cause a failure for Mac.

I rebased the PR and also trying an empty commit here to see if this failure is happening on the beta branch already.

@givip
Copy link

givip commented Dec 10, 2024

@christopherfujino I'm not sure how my web-only change could cause a failure for Mac.

I rebased the PR and also trying an empty commit here to see if this failure is happening on the beta branch already.

Can we merge?

@christopherfujino christopherfujino added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 10, 2024
@auto-submit auto-submit bot merged commit 83bacfc into flutter:flutter-3.27-candidate.0 Dec 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@yjbanov yjbanov yjbanov approved these changes

@ditman ditman ditman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mdebbar @givip @christopherfujino @yjbanov @ditman