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.

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

Various dysfunctional behaviors when using with shadow DOM (potential fix) #327

Open
datori opened this issue Aug 15, 2018 · 2 comments
Open

Comments

@datori
Copy link

datori commented Aug 15, 2018

Hello, I've been working on getting Squire working within a Polymer application, but I initially ran into a couple problems. Primary issues included the application failing to properly track cursor location, similar issues with text editing, and several issues vaguely related to focus. After tinkering with squire-raw.js for a while, it seems like the issue was being caused by two functions in particular, Squire.prototype.fireEvent and getWindowSelection.

With Squire.prototype.fireEvent, the line isFocused = this._root === this._doc.activeElement; is referencing the activeElement on this._doc, which is a direct reference to the document node. The issue arises because shadow DOMs have their own activeElement property, and therefore the shadow root that a given node is attached to is the only one capable of directly observing that node. When a node nested within a shadow DOM is active, the document can only see that the shadow host of that node is active, and thus the focus check fails since it compares the nested node to the 'active' shadow host. My workaround has been to replace the aforementioned line with isFocused = this._root === this._root.getRootNode().activeElement, which instead references the activeElement of the node to which the element is directly attached.

Similarly, the issue with getWindowSelection arises because it references the getSelection method of self._win, when it should reference the shadow root's. My solution was to replace the implementation with return self._root.getRootNode().getSelection() || null;

Thanks, and let me know if I missed anything.

@TehShrike
Copy link

TehShrike commented Jan 30, 2024

Thank you, this helped me quite a bit!

diff --git a/apps/editor-client/src/lib/squire/Editor.ts b/apps/editor-client/src/lib/squire/Editor.ts
index 67a1fae6f..4d438eed5 100644
--- a/apps/editor-client/src/lib/squire/Editor.ts
+++ b/apps/editor-client/src/lib/squire/Editor.ts
@@ -377,7 +377,7 @@ class Squire {
         // an infinite loop. So we detect whether we're actually
         // focused/blurred before firing.
         if (/^(?:focus|blur)/.test(type)) {
-            const isFocused = this._root === document.activeElement;
+            const isFocused = this._root === (this._root.getRootNode() as unknown as DocumentOrShadowRoot).activeElement;
             if (type === 'focus') {
                 if (!isFocused || this._isFocused) {
                     return this;
@@ -599,8 +599,13 @@ class Squire {
     }
 
     getSelection(): Range {
-        const selection = window.getSelection();
         const root = this._root;
+
+        const rootNode = root.getRootNode() as unknown as Document;
+        // In Firefox you have to use document.getSelection, in Chromium you have to use rootNode.getSelection
+        const objectWithGetSelection = 'getSelection' in rootNode ? rootNode : document;
+        const selection = objectWithGetSelection.getSelection() || null;
+
         let range: Range | null = null;
         // If not focused, always rely on cached selection; another function may
         // have set it but the DOM is not modified until focus again

@TehShrike
Copy link

Found one other shadow-dom-related bug in getSelection, here's the fix (assuming you've already applied the patch from my previous comment):

--- a/libs/squire/src/Editor.ts
+++ b/libs/squire/src/Editor.ts
@@ -627,7 +627,7 @@ class Squire {
             range = this._lastSelection;
             // Check the editor is in the live document; if not, the range has
             // probably been rewritten by the browser and is bogus
-            if (!document.contains(range.commonAncestorContainer)) {
+            if (!rootNode.contains(range.commonAncestorContainer)) {
                 range = null;
             }
         }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants