search

LibreTexts / ckeditor-binder-plugin

A CKeditor plugin that makes adding binder enabled pre tags easy.
GNU General Public License v3.0
4 stars 2 forks source link

CKEditor does not work on Microsoft Edge #108

Closed sandertyu closed 4 years ago

sandertyu commented 4 years ago

I'm not 100% certain that this is not just my client, but CKEditor seems to be entirely inoperable in Microsoft Edge. I went ahead and tested a barebones Thebe document in Edge as well, and while the code cells do appear normal, the kernel refuses to connect and no code can be ran. The same thing happens on official Thebe example pages as well, so opening an issue there is likely necessary. We should first get someone else to confirm that Thebe is broken on Edge first though I think.

Here is a bugged page/console output in Libretexts broken libretexts

And here is the console output on Thebe broken thebe

rkevin-arch commented 4 years ago

Culprit: https://github.com/LibreTexts/Libretext/blob/9d8decaaacc53cfecce577e3778c97cd286a1aa6/public/Miscellaneous/reuse.js#L130 Reason: https://stackoverflow.com/questions/53628191/edge-script1028-expected-identifier-string-or-number

@Miniland1333 This seems like it's in your ballpark. Do you mind taking a look?

rkevin-arch commented 4 years ago

Searching in the libretext repo I found a couple of other occurrences of that ...variable syntax. It might be a good idea to fix them all or add an explicit warning that edge users should switch to another browser.

Miniland1333 commented 4 years ago

What version of Edge are you using? Windows auto-updated mine the Chromium Edge so it looks like it is a different version than what you have. Chromium edge supports object spread and has been out for almost a year as a replacement for legacy Edge.

The stackoverflow issue you linked is almost two years old and browser support for object spread has increased a lot since then, which is why I'm curious about what browser you are using. ES2018 was released in 2018 and all semi-modern browsers support this.

And lastly, since the browser you are testing on is indeed experiencing the issue, I have done a basic restructure so that the code you care about (LibreEditor) is called first before the error appears. If you do a CTRL+F5 and it still fails, let me know and I can try a more aggressive fix.

rkevin-arch commented 4 years ago

To be fair, I haven't been using my Windows OS sine forever, but it should be decently up to date with security patches (at most one or two months off), so either Edge doesn't autoupdate with Windows or something else is going on here. I'll double check in a bit, but it's also nice to give users a bit of warning that their Edge isn't supported.

sandertyu commented 4 years ago

I believe my Edge version is below; edge version I didn't know Chromium Edge exists, it seems to have not downloaded by default on my Windows installation, which is at the latest version (2004, OS build 19041.508). I'm not personally sure whether this version of Edge is worth supporting, but if it's an easy fix then I guess you could go ahead and try. What you currently changed does not seem to have fixed the issue though unfortunately, with all the same console errors.

sandertyu commented 4 years ago

We met with Henry and found that Thebe just simply will not work in older versions of Edge. We're implementing an alert to notify users of older Edge that pages with code cells will not work, and suggesting that they upgrade to Chromium Edge.

sandertyu commented 4 years ago

Closing this, as we've noted that Edge does not work and we've placed a warning when users access LibreText via Edge