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

Lists and the Grammarly extension do not work well together on Chrome #422

Closed
YoranBrondsema opened this issue Jun 20, 2016 · 20 comments
Closed

Comments

@YoranBrondsema
Copy link
Collaborator

Grammarly is a widely used extension for grammar correction. Unfortunately, it does not play well with lists in Mobiledoc Kit on Chrome. If you go to https://bustlelabs.github.io/mobiledoc-kit/demo/ and add a list, you will see that moving the cursor does not function well anymore (it jumps back to its initial position) and memory and CPU usage also goes up.

I'm investigating what it could be linked to and if something on our side can be done to make it play well with Grammarly, or if something needs to be done on their side.

@bantic
Copy link
Collaborator

bantic commented Jun 21, 2016

Thanks for reporting this. I suspect that there are competing mutation observers: grammarly modifies content in the mobiledoc-kit editor -> the editor notices the mutation -> editor rerenders (removing grammarly's added DOM) -> grammarly notices and modifies the content -> ...

I will investigate — perhaps the mutation observer on mobiledoc-kit's side can be less aggressive about its modifications.

@YoranBrondsema
Copy link
Collaborator Author

I looked at this more in-depth and I found some other issues, namely when performing a correction. Your intuition was right, both cases have to do with the way the mutations are handled by mobiledoc-kit.

Unfortunately, we have higher priority things to work on at HSTRY (kids are back at school!) so I found a quick "workaround" that disables Grammarly, until this is fixed. I documented this in #490 in case other users have the same problem.

@blacktaxi
Copy link

Hi all,

I'm a dev and the tech lead for the Grammarly extension. First of all, sorry that you and your users are having issues with Grammarly!

We've been working on a new way to integrate Grammarly on websites which specifically aims to solve compatibility issues with RTEs like Mobiledoc. This new tech is currently being rolled out as we continue to iron out issues, but it is not yet widely available.

I've just checked new Grammarly on the Mobiledoc demo page and the main things seem to work fine out of the box – underlines, replacements, etc. – which is great. There are some issues though.

Grammarly extension has the pop-up editor feature (shown when you click on the green G), which opens a separate editor that takes the most part of the screen. To implement that, Grammarly needs to be able to take the current text from the text field and then set the new text content of the text field after the Grammarly pop-up editor is closed. There are at least three ways to do it:

  1. read/write innerHTML of the contenteditable div
  2. read from innerHTML and write with a document.execCommand('insertHTML', ...) call
  3. emit fake ClipboardEvents to read and write the text

None of these are properly working with Mobiledoc. Do you guys have any suggestions on what would be a proper way to do it?

Another question is what is the most reliable way to detect a Mobiledoc editor? I can see that at least on the demo page the contenteditable div has a __mobiledoc-editor class. Is that a proper criterion?

And last but not least, please let me know if there are any other interesting/unique/unexpected features of Mobiledoc that we should be aware of so that Grammarly doesn't break them.

Thanks!

@mixonic
Copy link
Contributor

mixonic commented Jul 27, 2018

Howdy @blacktaxi, thanks for dropping in with a note. I'm going to try and bounce around some ideas with @bantic. Thanks for trying to improve how Grammerly interacts with JS on the page, you've got a challenge for sure.

Quick thoughts:

  • Mobiledoc-kit HTML/DOM is a reflection of the document but alone is no complex enough to reproduce the document. For example atoms and cards are POJO payloads rendered by app-specific JS. We do our best when HTML is pasted into the editor (or otherwise when it appears from an unexpected input path) to parse it back into a Mobiledoc AST, but reparsing of editor HTML is still lossy.
  • That said I'm working on making this better right now, but it is hard :-p
  • When you copy from one Mobiledoc to another we have a better implementation: Upon copy we stash the Mobiledoc in an HTML attribute, on the paste side we check for this and use it if present. But this is not really useful in the case Grammerly is dealing with.
  • We could probably change the DOM emitted by the editor to always container a JSON version of a card payload in that card's wrapped element. This would help us re-parse the card correctly if something edits and the submits the editors HTML back to it.
  • Mobiledoc-kit has a mutation observer which I would expect works with innerHTML= and execCommand('insertHTML', 🤔
  • We handle paste, so I am also surprised ClipboardEvent isn't doing anything 🤔
  • __mobiledoc-editor is private re: the __, but I don't expect to change it. That is probably a pragmatic choice.

Hope I have some more later! 🍻

@blacktaxi
Copy link

blacktaxi commented Jul 30, 2018

Hey @mixonic,

Thanks for your notes, this is very useful, as I don't know much about Mobiledoc. I wasn't even trying to make it work with cards and atoms TBH – for now it's just to make the basic formatted texts work.

What I encountered when trying to do various import/export of the content from Mobile doc, is that after importing the new text (via either innerHTML or execCommand) the result is a corrupted doc, which will contain extraneous content. For example, with a bulleted list, I got the first item of the list to be duplicated and added to the end of the list. With another try (sorry I don't exactly remember which was which), the editor got into a state where it was not possible to move the caret beyond a certain point and make edits.

With ClipboardEvent, I wasn't getting any content back when I was emitting a copy event. Perhaps paste works – will need to check that. For complete clarity, here's what the code looks like:

// NOTE: both getText and setText need the document's Selection already set appropriately

var getText = function(target) {
  var clip = new DataTransfer()
  var e = new ClipboardEvent('copy', { target: target, clipboardData: clip })
  target.dispatchEvent(e)
  return clip.getData('text/html')
}

var setText = function(target, html) {
  var clip = new DataTransfer()
  clip.setData('text/html', html)
  var e = new ClipboardEvent('paste', { target: target, clipboardData: clip })
  target.dispatchEvent(e)
}

I will not be actively working on this right now, but will appreciate any advice on Mobiledoc for when I come back to work on it.

@dmarman
Copy link

dmarman commented Dec 26, 2018

I know it's a bit off topic. But the Grammarly webapp in its own editor is eating my CPU quite a lot on Chrome. So I can guess that the desktop application will do the same since probably it's an electron app.

@blacktaxi is this a known issue?

I am a paying customer, but with these issue I'll rethink my subscription...

@blacktaxi
Copy link

Hi @dmarman, I have to agree that it is a bit off-topic, but since you mentioned it, I don't think I'm aware of any acute CPU consumption issues with the Grammarly web app. Yes, the desktop Grammarly is an electron app. Please feel free to submit your report to Grammarly support.

@gustawdaniel
Copy link

gustawdaniel commented Jan 19, 2019

Any perspectives on solving this issue? I love both Grammarly and Ghost that uses mobile-doc in his new editor. I would love to use both instantly. Which code should be changed - mobiledoc-kit or Grammarly?

@ManuelRauber
Copy link

Since august, Grammarly is supporting Rich Text. Does this may help now for the integration? https://www.grammarly.com/blog/rich-text-formatting-grammarly-editor/

@kevinansfield
Copy link
Collaborator

@ManuelRauber that's only announcing limited rich-text support in their own editor, it doesn't say anything about the plugin which would be a lot more difficult to support to in a generic fashion. Happy to be proved wrong though 🙂

@ManuelRauber
Copy link

@kevinansfield Right. I've no idea how one thing relates to the other. It was more an idea that something is happening regarding rich-text support. So hopefully, it will come available in the plugin.

@oreniko26
Copy link

Hey @blacktaxi! If I wanted to help out in some way, could you provide a pointer on where to begin?

@kamranayub
Copy link

kamranayub commented Mar 11, 2021

If you're coming from Google or searching for enabling Grammarly for Ghost, like I was, figured this is the thread to post a workaround in. I made a quick and dirty user script that will enable Grammarly automatically whenever you're in the editor.

I made a video!

https://youtu.be/N5bljLQTMhc

Raw file and instructions: https://gist.github.com/kamranayub/191ae27ade85d19c7ec63aa1b1ebf499

It can be used with a userscript extension like TamperMonkey. 🙈

As mentioned in TryGhost/Ghost#11252 and this thread, it doesn't work as well copying text to/from the Grammarly editor to the post, but it works great for editing inline in the post.

@maikudou
Copy link

maikudou commented Jun 2, 2021

Hey. I am an engineer for Grammarly extension.
We changed a lot since this issue was posted.
I checked and our current implementation works good with mobiledoc. Replacements in lists and other stuff too.

It might not appear on https://bustlelabs.github.io/mobiledoc-kit/demo/ because the field initially is too small (we do not work on fields where we can't fit into, now threshold is 35px height, fields on the site are 33px)

You can check if you add a couple of lines, making the field bigger and then focus out and in again.

Feel free to ping me if you still have any questions or suggestions

About Ghost. I see their editor is behind a paywall, so I can't check it. @kamranayub, so you know if they use mobiledoc? I don't see "data-gramm" in mobiledoc demo page, so I am curious if they add it on their side.

@kamranayub
Copy link

@maikudou yes, they use mobiledoc and they add it. The source code is public and if you search TryGhost/ghost-admin I think you can see the data attribute. So there's a Chrome extension that removes it and my little user script also removes it.

@maikudou
Copy link

maikudou commented Jun 2, 2021

@bantic Can we have this checked and closed?
From Grammarly side we don't put anything in the field anymore (for a couple of years) and it should work well.
I don't observe high CPU usage or any other major problems.

@kevinansfield
Copy link
Collaborator

We're enabling the Grammarly extension in our next release of Ghost. We'll monitor through our support channels and if everything seems OK after a few weeks I'm happy to close this issue.

@YoranBrondsema
Copy link
Collaborator Author

Just a reminder that when the issue is closed, the paragraph in the README should also be removed:

image

@YoranBrondsema
Copy link
Collaborator Author

Also just an FYI that we removed the "data-gramm" on all the mobiledoc-kit instances on Sutori, and the extension is working well. Thanks @maikudou!

@tholman
Copy link
Contributor

tholman commented Sep 22, 2021

🎉

@tholman tholman closed this as completed Sep 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests