From 33b4f9b27e6209cf533da041abd7066928947f64 Mon Sep 17 00:00:00 2001 From: Alberto Fernandez-Capel Date: Mon, 2 Oct 2023 12:18:56 +0100 Subject: [PATCH 1/3] Always rely on the selectionchange API This removes the fallback to polling for selection changes. The selectionchange API has ben supported by all browsers that Trix supports for a long time now. The last browsers to add support for it was Firefox in version 52, and Chrome for Android, both in 2017. --- src/trix/observers/selection_change_observer.js | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/trix/observers/selection_change_observer.js b/src/trix/observers/selection_change_observer.js index ba0c1aa83..82e6be508 100644 --- a/src/trix/observers/selection_change_observer.js +++ b/src/trix/observers/selection_change_observer.js @@ -7,18 +7,13 @@ export default class SelectionChangeObserver extends BasicObject { constructor() { super(...arguments) this.update = this.update.bind(this) - this.run = this.run.bind(this) this.selectionManagers = [] } start() { if (!this.started) { this.started = true - if ("onselectionchange" in document) { - return document.addEventListener("selectionchange", this.update, true) - } else { - return this.run() - } + document.addEventListener("selectionchange", this.update, true) } } @@ -61,15 +56,6 @@ export default class SelectionChangeObserver extends BasicObject { this.domRange = null return this.update() } - - // Private - - run() { - if (this.started) { - this.update() - return requestAnimationFrame(this.run) - } - } } const domRangesAreEqual = (left, right) => From 1a66aa08624ac29f8c8cde35f83524863a5972a6 Mon Sep 17 00:00:00 2001 From: Alberto Fernandez-Capel Date: Mon, 2 Oct 2023 15:29:15 +0100 Subject: [PATCH 2/3] Remove check for DOM range equality in SelectionChangeObserver This checks causes problems on Safari 17 when you drag the cursor to select text. Safari weirdly reports the selection range as being collapsed, even when you select multiple characters. This causes the SelectionManager to think that the selection is collapsed and not trying to find where it ends. The end result for the user is that they can not apply formatting or links to the whole selected text. This is ultimately a Safari bug, but it's not clear that we need to check for DOM range equality in the first place. The only reason we do this is because the original implementation of the SelectionChangeObserver used polling to check for selection changes, instead of relying on the `selectionchange` event. Now that we're using the `selectionchange` event, we can assume that if a selection change event fires, the selection has changed. --- src/trix/observers/selection_change_observer.js | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/src/trix/observers/selection_change_observer.js b/src/trix/observers/selection_change_observer.js index 82e6be508..c9b2193eb 100644 --- a/src/trix/observers/selection_change_observer.js +++ b/src/trix/observers/selection_change_observer.js @@ -43,27 +43,14 @@ export default class SelectionChangeObserver extends BasicObject { } update() { - const domRange = getDOMRange() - const caretMove = window.getSelection().type === "Caret" - - if (!domRangesAreEqual(domRange, this.domRange) || caretMove) { - this.domRange = domRange - return this.notifySelectionManagersOfSelectionChange() - } + this.notifySelectionManagersOfSelectionChange() } reset() { - this.domRange = null - return this.update() + this.update() } } -const domRangesAreEqual = (left, right) => - left?.startContainer === right?.startContainer && - left?.startOffset === right?.startOffset && - left?.endContainer === right?.endContainer && - left?.endOffset === right?.endOffset - export const selectionChangeObserver = new SelectionChangeObserver() export const getDOMSelection = function() { From 57312b81e62186a5e7d2ab9f5e44068802abf9f9 Mon Sep 17 00:00:00 2001 From: Alberto Fernandez-Capel Date: Mon, 2 Oct 2023 15:37:54 +0100 Subject: [PATCH 3/3] Fix style violation --- src/trix/observers/selection_change_observer.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/trix/observers/selection_change_observer.js b/src/trix/observers/selection_change_observer.js index c9b2193eb..b8d78152e 100644 --- a/src/trix/observers/selection_change_observer.js +++ b/src/trix/observers/selection_change_observer.js @@ -1,6 +1,3 @@ -/* eslint-disable - id-length, -*/ import BasicObject from "trix/core/basic_object" export default class SelectionChangeObserver extends BasicObject { @@ -32,7 +29,7 @@ export default class SelectionChangeObserver extends BasicObject { } unregisterSelectionManager(selectionManager) { - this.selectionManagers = this.selectionManagers.filter((s) => s !== selectionManager) + this.selectionManagers = this.selectionManagers.filter((sm) => sm !== selectionManager) if (this.selectionManagers.length === 0) { return this.stop() }