From fb14c2dec97bcc4884e5abf123cb0914719afd4f Mon Sep 17 00:00:00 2001 From: Samuel Mannehed Date: Mon, 21 Oct 2019 11:26:40 +0200 Subject: [PATCH] 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 Co-authored-by: Niko Lehto --- kasmweb/core/util/cursor.js | 29 +++++++++++++++++++++++++++++ kasmweb/core/util/events.js | 37 ++++++++++++++++++++----------------- 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/kasmweb/core/util/cursor.js b/kasmweb/core/util/cursor.js index cd6b72c..be7ce26 100644 --- a/kasmweb/core/util/cursor.js +++ b/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); + } } diff --git a/kasmweb/core/util/events.js b/kasmweb/core/util/events.js index 7700712..daaed82 100644 --- a/kasmweb/core/util/events.js +++ b/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();