Mousemove fires outside canvas

[Riley-Howley]

Description

Fixes Issue: #12373
This PR addresses adding a bounds check on the ScreenSpaceEventHandler handleMouseMove this issue was only reproducible in Firefox. Checking to make sure the client is inside the bounds of the canvas solves this issue

When integration Cesium in existing application, if menus are overlayed on the canvas then in firefox the event is triggered even though its not in the canvas

Issue number and link

#12373

Testing plan

Using the given example in the Issue, after my change I tested in Firefox, and Chrome. Note: I did this on a Mac.

Automated tests were updated to have a valid boundingClientRect so that it passes the new check. Also added a new test to ensure the event is not fired when in the bounds of the canvas

Author checklist

• I have submitted a Contributor License Agreement
• I have added my name to CONTRIBUTORS.md
• I have updated CHANGES.md with a short summary of my change
• I have added or updated unit tests to ensure consistent code coverage
• I have updated the inline documentation, and included code examples where relevant
• I have performed a self-review of my code

[github-actions]

Thank you for the pull request, @Riley-Howley!

✅ We can confirm we have a CLA on file for you.

[ggetz]

Thanks for the PR @Riley-Howley!

@jjspace Could you please revie?

[jjspace]

This ended up being an annoyingly deep rabbit hole to dig into. I think your solution does directly fix the original problem in the issue but I don't think it's actually the true cause of confusion.

The problem originally comes from the registerListeners function which checks whether the browser supportsPointerEvents and if it does not it sets up a set of mouse move events instead of pointer events. This was turned off for Firefox completely because their early implementation of pointer events didn't work correctly, see #6372. However these events behave differently which is causing the change in behavior between browsers.

When using mouse events we attach the mousemove and mouseup events to the full document instead of the canvas specifically so we can handle events when dragging outside of the canvas. I think this is (and should stay) the desired behavior because it feels really bad if I click inside the viewer and drag outside of it and have the drag canceled immediately when I leave the viewer. This is not a problem with pointer events because after a pointerdown event pointermove events are "re-targeted" into the target of the pointerdown (the canvas). This lets the drag behavior work as expected without attaching event handlers to the document.

1. I think we can enable firefox to use pointer events instead of mouse move events which are simpler and behave more how we'd expect Re-enable PointerEvents in Firefox once they fix their implementation #7003
2. When registerListeners does register the set of listeners for a browser that does not support pointer events it should behave completely the same as when it does. This means adding a check similar to the one you already did but being more selective for when it triggers
   • Mouse move events that are outside the canvas while currently part of a drag event should still trigger
   • Mouse move events that are outside the canvas when not part of a drag event should not trigger the handler

Hopefully this all makes sense, happy to talk through things more if you want

[jjspace]

This type of check technically works but isn't the behavior I'd actually expect. This will still trigger when hovering over any elements that overlap with the viewer which is common in our sandcastles and other viewer contexts. Even things like the builtin timeline and geocoder would still trigger with this check because it's purely based on the bounds.

I think a more accurate check is whether event.target !== screenSpaceHandler._element which more directly matches the behavior of the event being attached directly to the canvas

Check out this sandcastle with an "overlay element" that I would not expect the mouse move handler to fire over.

[jjspace]

Also it looks like linting is failing. Please run npm run prettier and also make sure your git hooks are set up to correctly lint all code before it's pushed up to avoid CI failing.
Edit: this was not your fault, just different versions of prettier getting installed (see #12481). If you merge in main the linting error should be fixed.

You also have some spec failures, please check those after any other changes you make