Fix disappearing cursor after click

In the cursor emulation when deciding if the cursor should be hidden -
Instead of checking what's under the cursor, we check the element that
has capture.

This introduced another bug in the cursor emulation. The cursor did not
always disappear properly when using our cursor emulation together with
our setCapture polyfill. More specifically, we saw a problem when a
capture ended on an element without cursor emulation.

We solved this by introducing another visibility check on a timer in
the cursor emulation. However this led to yet another problem where
this timer conflicted with the timer in the setCapture polyfill.

We removed the timeout in the setCapture polyfill and created a
variable to make sure that all the events remaining in the queue can be
completed.

Co-authored-by: Alex Tanskanen <aleta@cendio.se>
Co-authored-by: Niko Lehto <nikle@cendio.se>

kasmweb/core/util/cursor.js

@@ -151,6 +151,25 @@ export default class Cursor {
         // now and adjust visibility based on that.
         let target = document.elementFromPoint(event.clientX, event.clientY);
         this._updateVisibility(target);
+
+        // Captures end with a mouseup but we can't know the event order of
+        // mouseup vs releaseCapture.
+        //
+        // In the cases when releaseCapture comes first, the code above is
+        // enough.
+        //
+        // In the cases when the mouseup comes first, we need wait for the
+        // browser to flush all events and then check again if the cursor
+        // should be visible.
+        if (this._captureIsActive()) {
+            window.setTimeout(() => {
+                // Refresh the target from elementFromPoint since queued events
+                // might have altered the DOM
+                target = document.elementFromPoint(event.clientX,
+                                                   event.clientY);
+                this._updateVisibility(target);
+            }, 0);
+        }
     }
 
     _handleTouchStart(event) {
@@ -208,6 +227,11 @@ export default class Cursor {
     }
 
     _updateVisibility(target) {
+        // When the cursor target has capture we want to show the cursor.
+        // So, if a capture is active - look at the captured element instead.
+        if (this._captureIsActive()) {
+            target = document.capturedElem;
+        }
         if (this._shouldShowCursor(target)) {
             this._showCursor();
         } else {
@@ -219,4 +243,9 @@ export default class Cursor {
         this._canvas.style.left = this._position.x + "px";
         this._canvas.style.top = this._position.y + "px";
     }
+
+    _captureIsActive() {
+        return document.capturedElem &&
+            document.documentElement.contains(document.capturedElem);
+    }
 }

kasmweb/core/util/events.js

@@ -21,7 +21,8 @@ export function stopEvent(e) {
 
 // Emulate Element.setCapture() when not supported
 let _captureRecursion = false;
-let _capturedElem = null;
+let _elementForUnflushedEvents = null;
+document.capturedElem = null;
 function _captureProxy(e) {
     // Recursion protection as we'll see our own event
     if (_captureRecursion) return;
@@ -30,7 +31,11 @@ function _captureProxy(e) {
     const newEv = new e.constructor(e.type, e);
 
     _captureRecursion = true;
-    _capturedElem.dispatchEvent(newEv);
+    if (document.capturedElem) {
+        document.capturedElem.dispatchEvent(newEv);
+    } else {
+        _elementForUnflushedEvents.dispatchEvent(newEv);
+    }
     _captureRecursion = false;
 
     // Avoid double events
@@ -50,17 +55,16 @@ function _captureProxy(e) {
 // Follow cursor style of target element
 function _capturedElemChanged() {
     const proxyElem = document.getElementById("noVNC_mouse_capture_elem");
-    proxyElem.style.cursor = window.getComputedStyle(_capturedElem).cursor;
+    proxyElem.style.cursor = window.getComputedStyle(document.capturedElem).cursor;
 }
 
 const _captureObserver = new MutationObserver(_capturedElemChanged);
 
-let _captureIndex = 0;
-
 export function setCapture(target) {
     if (target.setCapture) {
 
         target.setCapture();
+        document.capturedElem = target;
 
         // IE releases capture on 'click' events which might not trigger
         target.addEventListener('mouseup', releaseCapture);
@@ -92,8 +96,7 @@ export function setCapture(target) {
             proxyElem.addEventListener('mouseup', _captureProxy);
         }
 
-        _capturedElem = target;
-        _captureIndex++;
+        document.capturedElem = target;
 
         // Track cursor and get initial cursor
         _captureObserver.observe(target, {attributes: true});
@@ -112,21 +115,21 @@ export function releaseCapture() {
     if (document.releaseCapture) {
 
         document.releaseCapture();
+        document.capturedElem = null;
 
     } else {
-        if (!_capturedElem) {
+        if (!document.capturedElem) {
             return;
         }
 
-        // There might be events already queued, so we need to wait for
-        // them to flush. E.g. contextmenu in Microsoft Edge
-        window.setTimeout((expected) => {
-            // Only clear it if it's the expected grab (i.e. no one
-            // else has initiated a new grab)
-            if (_captureIndex === expected) {
-                _capturedElem = null;
-            }
-        }, 0, _captureIndex);
+        // There might be events already queued. The event proxy needs
+        // access to the captured element for these queued events.
+        // E.g. contextmenu (right-click) in Microsoft Edge
+        //
+        // Before removing the capturedElem pointer we save it to a
+        // temporary variable that the unflushed events can use.
+        _elementForUnflushedEvents = document.capturedElem;
+        document.capturedElem = null;
 
         _captureObserver.disconnect();