Skip to content
This repository has been 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
merged 2 commits into from
Dec 10, 2024
Merged

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

merged 2 commits into from
Dec 10, 2024

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
Member

@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
27 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@yjbanov yjbanov yjbanov approved these changes

@ditman ditman ditman approved these changes

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