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.

Sign up for GitHub

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

Jump to bottom

Broken in Shadow DOM #588

Closed
bedeoverend opened this issue Mar 16, 2017 · 12 comments
Closed

Broken in Shadow DOM #588

bedeoverend opened this issue Mar 16, 2017 · 12 comments

Comments

@bedeoverend
Copy link

Using a trimmed down version of the basic setup, the editor seems to break pretty heavily if it's created within Shadow DOM. There's a lot of errors being thrown, most seem to be triggered during selection. See here for demo of it happening - ensure the log is open, and it shows the errors being thrown. Also can't seem to backspace?

Browser: Chrome 56, Mac
Module Versions: All version 0.18.0

HTML Code:

<!doctype html>
<html>
  <head>
    <meta charset="utf-8"/>
    <style>
      .host {
       border: solid thin blue; 
      }
    </style>
  </head>
  <body>
    <div class="host"></div>
    <script src="index.js"></script>
  </body>
</html>

JS Code:

const {EditorState} = require("prosemirror-state")
const {EditorView} = require("prosemirror-view")
const {DOMParser} = require("prosemirror-model")
const {schema} = require("prosemirror-schema-basic")

let host = document.querySelector('.host'),
    editor;

host.attachShadow({ mode: 'open' });
host.shadowRoot.innerHTML = '<div class="editor"></div>';

editor = host.shadowRoot.querySelector('.editor');

window.view = new EditorView(editor, {
  state: EditorState.create({
    doc: DOMParser.fromSchema(schema).parse(editor)
  })
});
@marijnh
Copy link
Member

marijnh commented Mar 16, 2017

Does attached patch help? I couldn't see any issues with backspace, but I did notice the editor refusing to update its selection because it believed it wasn't focused. That should be fixed with this patch.

@marijnh
Copy link
Member

marijnh commented Mar 16, 2017

Err, I meant patch ProseMirror/prosemirror-view@5dfbaab

@bedeoverend
Copy link
Author

Hmm...patch doesn't seem to change anything for me, though I imagine probably still good to have in there. It looks like there are only issues when the cursor is at the end of the text string. For example, if I type 'Hello World' and the cursor is at the end, backspace and any arrow keys don't work. But if I click to somewhere in the middle I can use backspace / keys.

Looks like it's in skipIgnoredNodesLeft - where it's getting the selection, the anchorNode is null.

It's also coming up in skipIgnoredNodesRight, again because anchorNode is null.

I'm also getting errors in withFlushedState where it's trying to get a range that doesn't exist (0 is not a valid index) - but I'm assuming that's because things get confused after that first error? This error definitely only happens after the first one to do with the anchorNode.

@marijnh
Copy link
Member

marijnh commented Mar 16, 2017

Could you try upgrading to version 0.19.0 (which I just released) and testing again? Maybe I had some other fix locally, and that's why I didn't see the problems.

@bedeoverend
Copy link
Author

Ok so that seems to have fixed 99% of the issues - but seems like selection isn't working...so it seems to think any selection in Shadow DOM is empty. I've created another bin to demonstrate - I've just adding a plugin that just logs if selection is empty or not at every transaction. Running the same code on an editor that's in Light DOM works fine, but in Shadow the selection is always empty.

@bedeoverend
Copy link
Author

Looks like it's https://bugs.chromium.org/p/chromium/issues/detail?id=447523 that's causing the problem. Essentially means that domSel.isCollapsed always returns true. A potential workaround would be checking that it's collapsed, and then also checking its ranges are collapsed - domSel.isCollapsed && (domSel.rangeCount === 0 || domSel.getRangeAt(0).collapsed)).

With that workaround, selections seem to work, though there're some issues around focusing I believe...will continue investigating.

@bradleyayers - I noticed you'd run into Shadow DOM issues with Prosemirror, did this come up at all, did you have any solutions?

@bedeoverend bedeoverend mentioned this issue Mar 20, 2017
Merged
@bedeoverend
Copy link
Author

I've made a PR - that also fixes the focus issue I was having (same problem, just happening elsewhere). Let me know if you'd like me to change anything

@marijnh
Copy link
Member

marijnh commented Mar 20, 2017

Thanks for digging into that! I assume you are aware of #476, which still kind of ruins attempts to use ProseMirror in a shadow dom on Safari?

@bedeoverend
Copy link
Author

@marijnh no worries - thanks for ProseMirror 😃 Yeah I'm aware of that one, we're still using v0 Shadow DOM so don't run into the issue on Safari, but even the v1 Polyfills don't support it yet. I'm looking at ways to get around it - potentially a partial polyfill for that part of the spec, but won't be looking into it for awhile yet.

@bedeoverend
Copy link
Author

bedeoverend commented Apr 12, 2017 •
edited
Loading

@marijnh so looks like this is still broken - I mustn't have tested properly after this landed in a tagged release. Looks like this commit broke it, as I don't think there's a chrome prop in browser object?

@bedeoverend bedeoverend mentioned this issue Apr 12, 2017
Closed
@marijnh
Copy link
Member

marijnh commented Apr 13, 2017

Ah, apologies. Does patch ProseMirror/prosemirror-view@931e286 help?

@bedeoverend
Copy link
Author

Awesome - that'll do it, thanks @marijnh and no worries 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add chrome browser check
2 participants
@marijnh @bedeoverend