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

Does not work in Shadow DOM in Safari 10. #476

Open
bradleyayers opened this issue Oct 25, 2016 · 24 comments
Open

Does not work in Shadow DOM in Safari 10. #476

bradleyayers opened this issue Oct 25, 2016 · 24 comments

Comments

@bradleyayers
Copy link

When ProseMirror adopts an element inside a Shadow DOM in Safari 10, an exception is thrown.

The reason appears to be that in Safari 10 the extensions to DocumentOrShadowRoot don't appear to have all been applied to the shadow root.

This means that in rangeToDOM, getSelection is undefined, which when invoked throws an exception.

var sel = this.pm.root.getSelection(),

We've raised https://bugs.webkit.org/show_bug.cgi?id=163921 to track this against Safari.

@marijnh
Copy link
Member

marijnh commented Oct 25, 2016

Safari isn't really known for responding to bugs quickly. Is there a kludge you know of that lets us access the shadow dom's selection in another way in the meantime?

@bradleyayers
Copy link
Author

I haven't had time to look into this yet unfortunately.

In any case if it turns out that one does not exist, there are other kludges that might be tolerable on my side like using a polyfill implementation of Shadow DOM rather than native. I'll make sure to update this issue with any discoveries.

@marijnh
Copy link
Member

marijnh commented Mar 28, 2017

Has anyone checked whether the Safari shipped with 10.12.4 fixes this?

@bedeoverend
Copy link

Checked, still broken. That's Safari 10.1. Also still broken in Safari 10.2 (Tech Preview 26)

@madeleineostoja
Copy link

Looks like this might be an answer: https://github.com/GoogleChromeLabs/shadow-selection-polyfill

Could just flag in docs and encourage users to bundle the polyfill for safari. Note haven't tested it myself yet, was just released. Will spin up a fresh env to see if it resolves the issue when I get a mo.

@jsturgill
Copy link

jsturgill commented Nov 28, 2018

I'm just working my way through the very first introductory steps in your documentation in an attempt to get ProseMirror working in a web component using Shadow DOM and ran into this.

There may be things it doesn't address, but adding a ShadowRoot.getSelection method that simply calls document.getSelection appears to make things go so far.

I made an NPM package shadow-root-get-selection-polyfill for it.

@marijnh
Copy link
Member

marijnh commented Nov 29, 2018

Last time I looked, document.getSelection wouldn't accurately reflect selections inside a shadow DOM (by just treating the shadow root as being the selected node). Did that change?

@jsturgill
Copy link

jsturgill commented Nov 29, 2018

I didn't have Safari to test with, but looking into it more, I doubt it does anything helpful in Safari. The fix only works in Firefox because of a quirk in its implementation of ShadowRoot.

@alex-ketch
Copy link

Just encountered this issue with CodeMirror 6 prototype while trying to pass in a Shadow DOM to the editor root configuration.
Wanted to report that, at first glance, the jsturgill/shadow-root-get-selection-polyfill seems to have gotten the editor working in Safari 13.0.3 and Firefox 71.

@marijnh
Copy link
Member

marijnh commented Nov 22, 2019

Thanks, that's useful to know.

@marijnh
Copy link
Member

marijnh commented Mar 18, 2020

ProseMirror/prosemirror-view@bf61fc34b74baf2fb7884fa might help with this. (And if not, I don't believe there anything ProseMirror can do about this, so I'm closing this issue.)

@marijnh
Copy link
Member

marijnh commented Apr 21, 2021

On closer thought, no, that patch definitely won't help with this. Did anyone have any success getting ProseMirror to work in a shadow root in Safari?

@marijnh
Copy link
Member

marijnh commented May 1, 2021

We found a (somehow horrible) kludge for this in CodeMirror 6 that seems to mostly work. If this is still important for someone, we could look into porting that to ProseMirror.

@cymptom
Copy link

cymptom commented Jun 4, 2021

We found a (somehow horrible) kludge for this in CodeMirror 6 that seems to mostly work. If this is still important for someone, we could look into porting that to ProseMirror.

I'd be interested in that. I really don't want to drop shadow DOM for my rich text editor component just to support Safari, but as of now it's looking like I'm gonna need to 😞

@jpzwarte
Copy link

@marijnh Would love to see this in ProseMirror. Porting out ProseMirror-based Angular component to Web Component (with Shadow DOM) is on my TODO list :)

@jpduckwo
Copy link

jpduckwo commented Jun 17, 2022

I think we just ran into this issue too with Angular and ProseMirror

@leogreu
Copy link

leogreu commented Aug 23, 2022

@marijnh Are there any updates on this? We are having the same problem, unfortunately 😕

marijnh added a commit to ProseMirror/prosemirror-view that referenced this issue Sep 30, 2022
FIX: Work around the five-year-old Safari bug where it won't accurately report
the selection inside shadow roots, which would break ProseMirror when put
in shadow DOM.

Issue #142
Issue ProseMirror/prosemirror#476
@marijnh
Copy link
Member

marijnh commented Sep 30, 2022

Could people who are interested in this issue test the current master branch?

@roman-kulakov
Copy link

Hey @marijnh, thank you for this fix, I have checked demo with shadow dom and for me all works as expected. Could you pls publish a patch for this.

@marijnh
Copy link
Member

marijnh commented Oct 5, 2022

I've tagged this as 1.28.3

@Dhananjeyan286
Copy link

Hey @marijnh , can't we have a function like setRoot() that sets the root of the prosemirror dynamically?

We have a requirement where we have initialisied prosemirror within shadowRoot and at some point in time we need to bring it outside the shadowRoot to the body. So in these cases, if we have some function like setRoot(), it will be easier. Is there any technical difficulty in implementing this?

marijnh added a commit to ProseMirror/prosemirror-view that referenced this issue Sep 27, 2023
FEATURE: The new `EditorView.updateRoot` method can be used to make the editor
update its DOM root when it is moved to a new document or shadow tree.

Issue ProseMirror/prosemirror#476
@marijnh
Copy link
Member

marijnh commented Sep 27, 2023

Could you see if attached patch (which simply sets view._root to null, forcing a recomputation on the next access to .root).

@Dhananjeyan286
Copy link

It works on basic level of testing, thanks for the quick fix @marijnh 👍 .

@marijnh
Copy link
Member

marijnh commented Sep 27, 2023

Tagged as 1.32.0

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