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

fix(pointers): Fix possible missing pointer exited event with touch using wasm on iOS/iPad #18933

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(pointers): Fix possible missing pointer exited event with touch using wasm on iOS/iPad #18933

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
34140db
fix(pointers): Fix possible missing pointer exited event with touch u…
dr1rrb Nov 26, 2024
750041f
chore: Code review
dr1rrb Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions src/Uno.UI/Runtime/BrowserPointerInputSource.wasm.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using _NativeMethods = __Windows.UI.Core.CoreWindow.NativeMethods;
using System.Runtime.InteropServices;
using Windows.System;
using Microsoft.UI.Xaml;

namespace Uno.UI.Runtime;

Expand All @@ -29,6 +30,7 @@
private static readonly Logger? _logTrace = _log.IsTraceEnabled(LogLevel.Trace) ? _log : null;

private ulong _bootTime;
private bool _isIOs;
private bool _isOver;
private PointerPoint? _lastPoint;
private CoreCursor _pointerCursor = new(CoreCursorType.Arrow, 0);
Expand All @@ -55,11 +57,19 @@
private static partial void Initialize([JSMarshalAs<JSType.Any>] object inputSource);

[JSExport]
private static void OnInitialized([JSMarshalAs<JSType.Any>] object inputSource, double bootTime)
private static void OnInitialized([JSMarshalAs<JSType.Any>] object inputSource, double bootTime, string userAgent)
{
((BrowserPointerInputSource)inputSource)._bootTime = (ulong)bootTime;
if (inputSource is BrowserPointerInputSource that)
{
that._bootTime = (ulong)bootTime;
that._isIOs = userAgent.Contains("iPhone") || userAgent.Contains("iPad"); // Note: OperatingSystem.IsIOS() is false

Check notice on line 65 in src/Uno.UI/Runtime/BrowserPointerInputSource.wasm.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI/Runtime/BrowserPointerInputSource.wasm.cs#L65 

Change this call to 'userAgent.Contains' to an overload that accepts a 'StringComparison' as a parameter.
dr1rrb marked this conversation as resolved.
Show resolved Hide resolved

_logTrace?.Trace("Complete initialization of BrowserPointerInputSource, we are now ready to receive pointer events!");
_logTrace?.Trace("Complete initialization of BrowserPointerInputSource, we are now ready to receive pointer events!");
}
else if (_log.IsEnabled(LogLevel.Error))
{
_log.Error("Requested init using an invalid source.");
}
}

[JSExport]
Expand Down Expand Up @@ -121,6 +131,7 @@
case HtmlPointerEvent.pointerleave:
that._isOver = false;
that.PointerExited?.Invoke(that, args);
_PointerIdentifierPool.ReleaseManaged(pointerIdentifier);
break;

case HtmlPointerEvent.pointerdown:
Expand All @@ -130,7 +141,13 @@
case HtmlPointerEvent.pointerup:
//case HtmlPointerEvent.lostpointercapture: // if pointer is captured, we don't get a up, just a capture lost (with skia for wasm)
that.PointerReleased?.Invoke(that, args);
_PointerIdentifierPool.ReleaseManaged(pointerIdentifier);
if (that._isIOs && args is { CurrentPoint.PointerDeviceType: PointerDeviceType.Touch, DispatchResult: UIElement.PointerEventDispatchResult { VisualTreeAltered: true } })
{
// On iOS, when the element under the pointer is removed, the browser won't send any pointer leave event.

args.DispatchResult = null; // To be clean only, the value is not used in the leave case.
goto case HtmlPointerEvent.pointerleave;

Check warning on line 149 in src/Uno.UI/Runtime/BrowserPointerInputSource.wasm.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI/Runtime/BrowserPointerInputSource.wasm.cs#L149 

Remove this use of 'goto'.
}
break;

case HtmlPointerEvent.pointermove:
Expand Down
44 changes: 34 additions & 10 deletions src/Uno.UI/UI/Xaml/Internal/InputManager.Pointers.Managed.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,10 @@ private void OnPointerWheelChanged(Windows.UI.Core.PointerEventArgs args, bool i
#endif

var routedArgs = new PointerRoutedEventArgs(args, originalSource) { IsInjected = isInjected };
var result = default(PointerEventDispatchResult);
dr1rrb marked this conversation as resolved.
Show resolved Hide resolved

// First raise the event, either on the OriginalSource or on the capture owners if any
RaiseUsingCaptures(Wheel, originalSource, routedArgs, setCursor: true);
result = RaiseUsingCaptures(Wheel, originalSource, routedArgs, setCursor: true);

// Scrolling can change the element underneath the pointer, so we need to update
(originalSource, var staleBranch) = HitTest(args, caller: "OnPointerWheelChanged_post_wheel", isStale: _isOver);
Expand All @@ -250,7 +251,9 @@ private void OnPointerWheelChanged(Windows.UI.Core.PointerEventArgs args, bool i
// Second raise the PointerExited events on the stale branch
if (staleBranch.HasValue)
{
if (Raise(Leave, staleBranch.Value, routedArgs) is { VisualTreeAltered: true })
var leaveResult = Raise(Leave, staleBranch.Value, routedArgs);
result += leaveResult;
if (leaveResult is { VisualTreeAltered: true })
{
// The visual tree has been modified in a way that requires performing a new hit test.
originalSource = HitTest(args, caller: "OnPointerWheelChanged_post_leave").element ?? _inputManager.ContentRoot.VisualTree.RootElement;
Expand All @@ -259,14 +262,16 @@ private void OnPointerWheelChanged(Windows.UI.Core.PointerEventArgs args, bool i

// Third (try to) raise the PointerEnter on the OriginalSource
// Note: This won't do anything if already over.
Raise(Enter, originalSource!, routedArgs);
result += Raise(Enter, originalSource!, routedArgs);

if (!PointerCapture.TryGet(routedArgs.Pointer, out var capture) || capture.IsImplicitOnly)
{
// If pointer is explicitly captured, then we set it in the RaiseUsingCaptures call above.
// If not, we make sure to update the cursor based on the new originalSource.
SetSourceCursor(originalSource);
}

args.DispatchResult = result;
}

private void OnPointerEntered(Windows.UI.Core.PointerEventArgs args, bool isInjected = false)
Expand Down Expand Up @@ -300,7 +305,9 @@ private void OnPointerEntered(Windows.UI.Core.PointerEventArgs args, bool isInje

var routedArgs = new PointerRoutedEventArgs(args, originalSource) { IsInjected = isInjected };

Raise(Enter, originalSource, routedArgs);
var result = Raise(Enter, originalSource, routedArgs);

args.DispatchResult = result;
}

private void OnPointerExited(Windows.UI.Core.PointerEventArgs args, bool isInjected = false)
Expand Down Expand Up @@ -340,13 +347,16 @@ private void OnPointerExited(Windows.UI.Core.PointerEventArgs args, bool isInjec

var routedArgs = new PointerRoutedEventArgs(args, originalSource) { IsInjected = isInjected };

Raise(Leave, overBranchLeaf, routedArgs);
var result = Raise(Leave, overBranchLeaf, routedArgs);

if (!args.CurrentPoint.IsInContact && (PointerDeviceType)args.CurrentPoint.Pointer.Type == PointerDeviceType.Touch)
{
// We release the captures on exit when pointer if not pressed
// Note: for a "Tap" with a finger the sequence is Up / Exited / Lost, so the lost cannot be raised on Up
ReleaseCaptures(routedArgs);
}

args.DispatchResult = result;
}

private void OnPointerPressed(Windows.UI.Core.PointerEventArgs args, bool isInjected = false)
Expand Down Expand Up @@ -396,7 +406,9 @@ private void OnPointerPressed(Windows.UI.Core.PointerEventArgs args, bool isInje
var routedArgs = new PointerRoutedEventArgs(args, originalSource) { IsInjected = isInjected };

_pressedElements[routedArgs.Pointer] = originalSource;
Raise(Pressed, originalSource, routedArgs);
var result = Raise(Pressed, originalSource, routedArgs);

args.DispatchResult = result;
}

private void OnPointerReleased(Windows.UI.Core.PointerEventArgs args, bool isInjected = false)
Expand Down Expand Up @@ -439,7 +451,8 @@ private void OnPointerReleased(Windows.UI.Core.PointerEventArgs args, bool isInj

var routedArgs = new PointerRoutedEventArgs(args, originalSource) { IsInjected = isInjected };

RaiseUsingCaptures(Released, originalSource, routedArgs, setCursor: false);
var result = RaiseUsingCaptures(Released, originalSource, routedArgs, setCursor: false);

if (isOutOfWindow || (PointerDeviceType)args.CurrentPoint.Pointer.Type != PointerDeviceType.Touch)
{
// We release the captures on up but only after the released event and processed the gesture
Expand All @@ -451,6 +464,8 @@ private void OnPointerReleased(Windows.UI.Core.PointerEventArgs args, bool isInj
SetSourceCursor(originalSource);
}
ClearPressedState(routedArgs);

args.DispatchResult = result;
}

private void OnPointerMoved(Windows.UI.Core.PointerEventArgs args, bool isInjected = false)
Expand Down Expand Up @@ -482,11 +497,14 @@ private void OnPointerMoved(Windows.UI.Core.PointerEventArgs args, bool isInject
}

var routedArgs = new PointerRoutedEventArgs(args, originalSource) { IsInjected = isInjected };
var result = default(PointerEventDispatchResult);

// First raise the PointerExited events on the stale branch
if (staleBranch.HasValue)
{
if (Raise(Leave, staleBranch.Value, routedArgs) is { VisualTreeAltered: true })
var leaveResult = Raise(Leave, staleBranch.Value, routedArgs);
result += leaveResult;
if (leaveResult is { VisualTreeAltered: true })
{
// The visual tree has been modified in a way that requires performing a new hit test.
originalSource = HitTest(args, caller: "OnPointerMoved_post_leave").element ?? _inputManager.ContentRoot.VisualTree.RootElement;
Expand All @@ -495,14 +513,18 @@ private void OnPointerMoved(Windows.UI.Core.PointerEventArgs args, bool isInject

// Second (try to) raise the PointerEnter on the OriginalSource
// Note: This won't do anything if already over.
if (Raise(Enter, originalSource, routedArgs) is { VisualTreeAltered: true })
var enterResult = Raise(Enter, originalSource, routedArgs);
result += enterResult;
if (enterResult is { VisualTreeAltered: true })
{
// The visual tree has been modified in a way that requires performing a new hit test.
originalSource = HitTest(args, caller: "OnPointerMoved_post_enter").element ?? _inputManager.ContentRoot.VisualTree.RootElement;
}

// Finally raise the event, either on the OriginalSource or on the capture owners if any
RaiseUsingCaptures(Move, originalSource, routedArgs, setCursor: true);

args.DispatchResult = result;
}

private void OnPointerCancelled(PointerEventArgs args, bool isInjected = false)
Expand Down Expand Up @@ -535,10 +557,12 @@ private void OnPointerCancelled(PointerEventArgs args, bool isInjected = false)

var routedArgs = new PointerRoutedEventArgs(args, originalSource) { IsInjected = isInjected };

RaiseUsingCaptures(Cancelled, originalSource, routedArgs, setCursor: false);
var result = RaiseUsingCaptures(Cancelled, originalSource, routedArgs, setCursor: false);
// Note: No ReleaseCaptures(routedArgs);, the cancel automatically raise it
SetSourceCursor(originalSource);
ClearPressedState(routedArgs);

args.DispatchResult = result;
}

#region Captures
Expand Down
4 changes: 3 additions & 1 deletion src/Uno.UI/ts/Runtime/BrowserPointerInputSource.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@
this._bootTime = Date.now() - performance.now();
this._source = manageSource;

BrowserPointerInputSource._exports.OnInitialized(manageSource, this._bootTime);
var userAgent = navigator.userAgent || navigator.vendor || window.opera;

Check warning on line 57 in src/Uno.UI/ts/Runtime/BrowserPointerInputSource.ts

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI/ts/Runtime/BrowserPointerInputSource.ts#L57 

Identifier 'userAgent' is never reassigned; use 'const' instead of 'var'.

BrowserPointerInputSource._exports.OnInitialized(manageSource, this._bootTime, userAgent);
this.subscribePointerEvents(); // Subscribe only after the managed initialization is done
}

Expand Down
8 changes: 8 additions & 0 deletions src/Uno.UWP/UI/Core/PointerEventArgs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ internal PointerEventArgs(PointerPoint currentPoint, VirtualKeyModifiers keyModi
public IList<PointerPoint> GetIntermediatePoints()
=> new List<PointerPoint> { CurrentPoint };

#nullable enable
/// <summary>
/// Gets the dispatch result of this event, if any.
/// This is defined by the InputManager if the event goes though it.
/// </summary>
internal object? DispatchResult { get; set; }
#nullable restore

/// <inheritdoc />
public override string ToString()
=> $"{CurrentPoint} | modifiers: {KeyModifiers}";
Expand Down
Loading