ianstormtaylor / slate

A completely customizable framework for building rich text editors. (Currently in beta.)
http://slatejs.org
MIT License
29.94k stars 3.25k forks source link

Editor selection is always null in Internet Explorer 11 (IE11) even with additional polyfills #4111

Open marko-hologram opened 3 years ago

marko-hologram commented 3 years ago

Description Editor selection is always null in Internet Explorer 11 (IE11) and therefore no editing can be applied to any text. No error is observed in developer tools console during this operation, it just fails silently.

Recording IE11 behavior: https://user-images.githubusercontent.com/22904204/110108060-ffd59880-7dab-11eb-9832-0d3e0dd3b509.mp4

Chrome/Firefox etc. https://user-images.githubusercontent.com/22904204/110108332-55aa4080-7dac-11eb-9f36-38b7e2f9659c.mp4

Sandbox Unfortunately codesandbox doesn't work in IE11 so I cannot really use these online environments to create a minimal example. I created this small repository instead: https://github.com/marko-hologram/issue-slate-ie11

Steps To reproduce the behavior:

  1. Run the project as described in README
  2. Select any text inside the editor
  3. Press BOLD button or use Ctrl + B shortcut for example
  4. Observe that editor logged inside the console has selection: null all the time and bold hasn't been applied

Expectation

Environment

Sidenote This project uses next-transpile-modules to transpile slate and slate-react packages in order for them to be used in IE11 because otherwise the code fails.

Polyfills used are from this comment https://github.com/ianstormtaylor/slate/issues/3800#issuecomment-758472391, just without the TS types and adapted for Nextjs to run only in browser, not on server.


I tried poking around some code in order to try to figure out where the problem happens, but I didn't come up with anything. I checked some code where some Selection logic is being triggered and as far as I've seen this useEffect doesn't run after doing a text selection, it only runs on initial mount. https://github.com/ianstormtaylor/slate/blob/513771c82ad3c464bee3c7e3b73bbce189c6cf6e/packages/slate-react/src/components/editable.tsx#L149

I would really appreciate your help because unfortunately we still have to support IE11 for some time on this project I'm working on. We picked Slate as our choice because we loved its simplistic approach to schema (among other things) and we also saw that IE11 support is possible. It would be kinda devastating now if IE11 support is not achievable in any way because we already got the editor up and running fairly well and we are trying to resolve IE11 compatibility now in a single pass.

I would love to drop IE11 support on this project I'm working on, but unfortunately it's not possible yet and I'm not the one to decide that 😄

marko-hologram commented 3 years ago

I went digging a bit into this. The thing that resolved the selection null issue was changing this line from target.parentElement to target.parentNode.

https://github.com/ianstormtaylor/slate/blob/513771c82ad3c464bee3c7e3b73bbce189c6cf6e/packages/slate-react/src/plugin/react-editor.ts#L179

Unfortunately that wasn't enough to make all of this work. After that Immer threw an error about ES5 mode not being turned on. To resolve this, I forced immer v8 with Yarn resolutions and I also had to go into bundled code (this time in slate package) and import enableES5 method and call it immediately at the top of the bundle.

After resolving that, there were others errors where there was an attempt to call .getAttribute() on null in two places here:

https://github.com/ianstormtaylor/slate/blob/513771c82ad3c464bee3c7e3b73bbce189c6cf6e/packages/slate-react/src/plugin/react-editor.ts#L314

I just randomly replaced that with this in the bundled code:

var isStartAtZeroWidth = startEl ? !!startEl.getAttribute('data-slate-zero-width') : true;
var isEndAtZeroWidth = endEl ? !!endEl.getAttribute('data-slate-zero-width') : false;

Of course this isn't really working properly, but at least it gets rid of the error.

And after this another issue popped up with some ownerDocument error in scrollIntoView, so I had to comment that out.

After all of this, I could finally get some formatting to apply and I could bold the text by using the code in the provided test repo.

Unfortunately the code from that example returns true in isBoldMarkActive function when the cursor is on the same line and not when it's only inside the bold text, but that's not an IE11 issue here.

I tried all of these changes then in the project where I actually need all of this and unfortunately checking if "mark" is applied doesn't work. Code is something like this:

// type RichTextEditorMarkTypes = "bold" | "italic" | "underline" | "strikethrough";
isMarkActive(editor: ReactEditor, format: RichTextEditorMarkTypes) {
  const marks = Editor.marks(editor);
  return marks ? marks[format] === true : false;
}

Another issue that popped out is that applying bold after selecting the text backwards changes the selection to have start and end point be at the end of that selection (left end of selection when selecting from right to left). Selecting from left to right works fine.

https://user-images.githubusercontent.com/22904204/110334305-eb4f0580-8022-11eb-88b3-1d9e381c1486.mp4

It also looks like trying to toggle bold formatting after pressing Enter to create new row fails with this error:

Cannot resolve a Slate node from DOM node: [object HTMLSpanElement]

Unfortunately I'm not sure if I can actually resolve all of this. I would have to have better knowledge of how Slate works internally and just getting to know the codebase is a lot of work. It took me a lot of time to even track some of these.

I think I'm going to have to switch to something else because IE11 support is still pretty important on this project. I would maybe be satisfied with some kind of a degraded experience, but I'm not sure what will happen going forward because I still have some stuff to implement (like lists and links). Also, I'm getting errors so it's not just degraded experience, it's just not working as it should.

It would just be helpful to update the FAQ (https://docs.slatejs.org/general/faq#what-browsers-and-devices-does-slate-support) part of the docs that indicates that IE11 support can be achieved with a few polyfills, because after trying it I realized that this isn't really true. It would have saved me a lot of time if I had knew that IE11 requires more than polyfills. Of course I could have tested IE11 earlier to discover these issues sooner, so that one is on me 😄

I pushed my changes to the repo I provided in the original post and I used patch-package (https://www.npmjs.com/package/patch-package) to "patch" these changes I'm mentioning here in this comment, so everything can be seen in that repo (https://github.com/marko-hologram/issue-slate-ie11).

kamilkazmierczak commented 3 years ago

I don't know if it helps but I had same issues as you with IE11 but polyfills that you mentioned helped me (I removed "as any" parts because my project is not in TS) I'm using slate 0.59.0 and I also added core-js 3.9.1 polyfills. But there is another issue with editor on IE but I made PR to fix that (https://github.com/ianstormtaylor/slate/pull/4118)

marko-hologram commented 3 years ago

I don't know if it helps but I had same issues as you with IE11 but polyfills that you mentioned helped me (I removed "as any" parts because my project is not in TS) I'm using slate 0.59.0 and I also added core-js 3.9.1 polyfills. But there is another issue with editor on IE but I made PR to fix that (#4118)

Hmm I don't know. I didn't have luck with all these polyfills and ran into problems as described above (not sure what I was doing wrong). Unfortunately I had to switch to something else and won't be using Slate anymore.

Contributors also said on Slack that they aren't really interested in worrying about IE11 support (which is fair of course), but it's just unfortunate for me. Even when I managed to resolve some issue with compatibility here, I then ran into another. This makes it a bit unreliable for me since I needed to implement more features and each of them could potentially break IE11 compatibility or whatever.

But still thanks for trying to help me 😄

thesunny commented 3 years ago

Updated the documentation with clarity around unresolved issues with IE11 even with polyfill.