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

Integration with Grammarly #267

Closed
fredck opened this issue Jul 26, 2016 · 44 comments
Closed

Integration with Grammarly #267

fredck opened this issue Jul 26, 2016 · 44 comments
Labels
resolution:expired This issue was closed due to lack of feedback. status:discussion status:stale type:feature This issue reports a feature request (an idea for a new functionality or a missing option). type:question This issue asks a question (how to...).
Milestone

Comments

@fredck
Copy link
Contributor

fredck commented Jul 26, 2016

Current status - 17.05.2021

Grammarly is compatible with CKEditor 5 and provides a good editing experience in the Chrome browser. 

Known issues: It's possible to experience some unstable behaviour while using the extension in Firefox.


I've recently installed Grammarly to check how it behaves on the CKEditor 5 demo.

Grammarly works with contenteditable enabled elements. I assume that it makes modifications to the DOM directly as corrections are made. In CKEditor 5, any correction you try to do with it is ignored.

To try it:

  1. Install the Grammarly extension for Chrome.
  2. Go to the CKEditor 5 demo.
  3. Type Lets go! at the end of the paragraph.

Expected:

  1. Grammarly should bring a suggestion for Lets but the underline highlight is not displayed.
  2. Try fixing it by clicking the Grammarly icon at lower right. Nothing will be changed in the editor.
@pjasiun
Copy link

pjasiun commented Jul 26, 2016

I also use Grammarly and recently with @szymonkups we were checking why one manual test (http://tests.ckeditor.dev:1030/build/amd/tests/engine/view/manual/immutable) is failing on my computer. It appears that it was Grammarly who insert unexpected text node with a space between "foo" and "bar".

@pjasiun
Copy link

pjasiun commented Jul 26, 2016

So it is not only about making external manipulation (Grammarly) working fine with CKEditor, but also about CKEditor working fine with external manipulation.

@Reinmar
Copy link
Member

Reinmar commented Jul 26, 2016

I can see that Grammarly doesn't work with any of the new editors (ProseMirror, Draft.js) and it's not a surprise. You must not carelessly modify someone's DOM and Grammarly creates its own spans in the content.

Currently, CKEditor 5 totally blocks Grammarly which is a nice behaviour. For instance, Draft.js and Slate totally breaks when Grammarly is used. CKEditor works fine thanks to the editing engine architecture, which simply rejects mutations which don't have handlers.

Also, thanks to the architecture we could think of actually supporting Grammarly by one of these ways:

  • Adding support for marking elements in the DOM as "transparent" for the DOM converter. We'd need to ask Grammarly to start using some attribute on these spans so they are discoverable. However, despite having a very narrow effect on our codebase, this will be a tricky thing to implement in the DOM converter. The algorithms there will become a loooot more tricky, but OTOH being able to add a transparent markup to the content would be an awesome feature.
  • By using model attributes Grammarly would be able to create a very simple plugin integrating it with CKEditor 5. Approaching integration from this side is much easier, but it'd require interaction with Grammarly. Also, it's not clear how such feature should be shipped – whether the Grammarly plugin for the browser should discover CKEditor instances and interact with them differently (hard to imagine how that could work) or whether it should be a plugin for CKEditor 5 integrating it with Grammarly (easy, but requires awareness and an action from a developer using CKEditor 5).

@Reinmar Reinmar added type:question This issue asks a question (how to...). status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option). labels Jul 26, 2016
@scofalik
Copy link
Contributor

scofalik commented Jul 26, 2016

I support @Reinmar on this.

We can't expect that every plugin doing anything with our editable contents works fine. I was about to ask how it works in other editors because I was not suspecting it to work without any additional integration.

However, I suppose that it should work with "ACF=true" converter, that is the converter that converts all unrecognized elements and attributes to the same elements and attributes in model.

I also think it would be the best if we had integration with Grammarly (or any other 3rd party service) done on our terms, using possibilities and functions that we created by introducing all the model<->converters<->view architecture.

@Reinmar
Copy link
Member

Reinmar commented Jul 26, 2016

That converter would need to handle mutations caused by adding those spans. This is something more than just converting any DOM to the model and back. You also need to be aware of how that DOM is being created.

@scofalik
Copy link
Contributor

scofalik commented Jul 26, 2016

Keep in mind, that Grammarly plugin would not have to modify model but view. Converting from view to DOM is easy, then, but it would need some plugin to handle adding extra view spans and clearing them. Operating on view is more similar to operating on DOM than operating on model. And you would not need to introduce converters.

I am not sure how it would work if you would remove some letters from a span added by Grammarly, but I have high hopes that Mapper would take care of it and it would be doable.

@Reinmar
Copy link
Member

Reinmar commented Jul 26, 2016

screen shot 2016-07-26 at 15 14 57

@fredck
Copy link
Contributor Author

fredck commented Feb 1, 2017

I'm in touch right now with the Grammarly support to understand what could be done in such case.

If there will be no useful outcome out of this contact, it seems that it is doable to disable Grammarly in a specific element by adding this attribute to it: data-gramm="false". I didn't test it though.

@pjar
Copy link

pjar commented Mar 13, 2017

@fredck using data-gramm="false" seems to work for me. It is a sad solution though.

@fredck
Copy link
Contributor Author

fredck commented Mar 14, 2017

Thanks for the confirmation, @pjar!

I'm still waiting for Grammarly to give any feedback on this, though. I'll ping them once again.

@fredck
Copy link
Contributor Author

fredck commented Mar 16, 2017

Ok, here is the log of my contact with the Grammarly support so far:

  • Feb 1 => problem exposed, containing technical details and proposal to solve this by disabling Grammarly in CKEditor 5 automatically by sniffing the editable classes. I've asked not for an immediate solution, but for their position and eventually their action plan.
  • Feb 1 <= reply from the support person that decided to not pass this to the technical team and tried to reply by herself. Misleading, unfortunately.
  • Feb 1 => I clarify the misunderstanding to her and ask her again to pass this to the technical team.
  • Feb 1 <= "I have forwarded your request over to our technical team. They will prioritize the issue and get in contact with you for further investigation."
  • Feb 23 => no contact so far, I ping them.
  • Feb 23 <= "I have forwarded your email to the corresponding department. We will get back to you shortly."
  • March 14 => no contact so far, I ping them again.
  • March 16 <= "The technical team is aware of the problem and they are currently working on it. Unfortunately, this will not be a quick fix and I cannot provide you with an estimated timeframe, as the developers will need some time to analyze the cause of this issue."

The conclusion is that.... we don't know. We don't know if this will be solved at their side. If so, then we don't know when and how. So, for now, we can just keep waiting.

@Reinmar
Copy link
Member

Reinmar commented Mar 16, 2017

We could also ping the guys (@A-gambit, @alexander-yakushev, @blacktaxi) directly. As we can see the "forwarding" didn't go particularly well, so we don't know what the team actually received. E.g. I'd be interested in hearing their opinion about the options that they consider viable. Having distorted one-way communication we may end up nowhere if we didn't understand each other correctly.

@pjasiun
Copy link

pjasiun commented Jul 26, 2017

Unfortunately, this issue is still open. We recently received some issues from users who use CKEditor 5 and Grammarly.

@pjasiun
Copy link

pjasiun commented Jul 26, 2017

By the way, this is related: https://github.com/grammarly/contenteditable

@Reinmar
Copy link
Member

Reinmar commented Jul 26, 2017

This is strange, Grammarly doesn't check spelling in Draft anymore:

image

@Reinmar
Copy link
Member

Reinmar commented Jul 26, 2017

Actually, on my Chrome, it doesn't check spelling on the entire website? I created a textarea and it didn't check the spelling in there too.

@Reinmar
Copy link
Member

Reinmar commented Jul 26, 2017

OK, we should read through facebookarchive/draft-js#616.

Edit: it was fixed on Grammarly side.

We released an update that skips all draft.js powered fields. Let me know if you still have somewhere this problem.

So we should contact them again.

@Reinmar Reinmar added this to the iteration 11 milestone Jul 26, 2017
@Reinmar
Copy link
Member

Reinmar commented Jul 26, 2017

I'm adding this to the current milestone so to remember to discuss this once again and take final actions.

@fredck
Copy link
Contributor Author

fredck commented Aug 9, 2017

I've contacted their support for yet another time. If we'll not have their attention still, I'm afraid we'll have to go the dirty way and use the data-gramm="false" attribute to block them. I'll let you guys know if I have news.

@blacktaxi
Copy link

Hi Frederico,

Thank you for bringing this up once again, and for your patience. And sorry it took us so long to respond.

For now, the right thing to do seems to be for Grammarly to not touch CKEditor DOM. We are going to implement and release this change in the coming weeks in Chrome. Safari and Firefox usually have a much longer release cycle because of the store review process, so you can expect a delay of additional two to four weeks there.

In the longer term, we would very much like to work in harmony with CKEditor. We're going to look into how we can make this happen. We don't currently have a timeline for this though.

I will keep you updated on our progress.

@Reinmar
Copy link
Member

Reinmar commented Aug 10, 2017

Hey @blacktaxi!

Thanks for the reply. Great to hear that you plan to work on this :)

How do you plan to check whether an editable element belongs to CKEditor?

We have two major versions of CKEditor right now – CKEditor 4 (which demo you can see on https://ckeditor.com) and CKEditor 5 (which demo you can see on https://ckeditor5.github.io). Both these editors use different classes to mark their editable elements. These are:

  • ck-editor__editable for CKEditor 5
  • cke_editable for CKEditor 4

There's also a question whether Grammarly needs to be disabled for CKEditor 4 too. It definitely cannot work with CKEditor 5 (due to its architecture), but CKEditor 4 is more forgiving. WDYT @mlewand and @fredck?

@fredck
Copy link
Contributor Author

fredck commented Aug 10, 2017

I think this is just for CKEditor 5. In CKEditor 4 the data model is the DOM itself, so changes made by external applications, especially the kinds of changes done by Grammarly, are usually fine.

@Reinmar
Copy link
Member

Reinmar commented Aug 10, 2017

Turns out that Grammarly may cause some issues in CKE 4 as well: https://dev.ckeditor.com/ticket/14332 and https://jira.xwiki.org/browse/CKEDITOR-117.

This is a special case where the full page mode is used so in most cases Grammarly may be working just fine.

@fredck
Copy link
Contributor Author

fredck commented Aug 10, 2017

Just want to be sure that Grammarly will be focused on fixing the situation with CKEditor 5 only asap. That's why I would avoid talking about CKEditor 4 at this point, because things should be ok there.

If we make the scope bigger, we make things slower.

@blacktaxi
Copy link

Hey @fredck, @Reinmar

Yes, it seems that Grammarly works with CKEditor 4 fine most of the time. This is why we haven't blacklisted it in the past. I also think that for now we should only focus on fixing the incompatibility with CKE 5. We can proceed with this plan and then see whether we need to do anything about CKE 4.

@Reinmar, we usually do check for definitive class names to discern different editors. Thanks for the tip!

@Reinmar
Copy link
Member

Reinmar commented Sep 6, 2017

I found one scenario when Grammarly is running on element with a ck-editor__editable class – this is when the class is added later.

The scenario looks like this:

  1. Create a page with a <div contenteditable> element.
  2. Grammarly discovers this element and starts following.
  3. Initialise CKEditor on this element.
  4. Grammarly is still present, despite the ck-editor__editable class being added in step 3.

Of course, a simple workaround is to not have <div contenteditable> elements in your page before you initialise CKEditor. I wonder, though, if there might be some reason to load HTML with an editable div from the beginning. To make it focusable (a11y?) before the editor is up? That could be solved with tabindex.

If we can't think of a reason to load such HTML, then it's simply not recommended to add the contenteditable attr by yourself.

@fredck
Copy link
Contributor Author

fredck commented Sep 6, 2017

The only use I can see for this is to mark elements that the system wants them to be RTEs. Sounds like a weird way for it though, as this can be done with classes or even custom attributes, without risking to give at the hands of end users a full contenteditable enabled, ready-to-be-messed-up element.

But still, I think I've seen something like this in the past... but let's wait for people to come with such cases before we think about doing anything about it.

@frattaro
Copy link

frattaro commented Sep 6, 2017

Might be good to document it in a best practices page: not recommended to use contenteditable as an identifier for containers pending RTE initialization.

I only butt in because that's how I'm doing it with CKEditor4

@Reinmar
Copy link
Member

Reinmar commented Sep 6, 2017

I only butt in because that's how I'm doing it with CKEditor4

Yeah... cause we're automatically hoisting elements which have cE attributes. That was a mistake :)

We're gonna add a note in the docs: https://github.com/ckeditor/ckeditor5-editor-inline/issues/27

@frattaro
Copy link

frattaro commented Sep 6, 2017

I ended up turning off automatic initialization for my use case, but still used the convention afterwards.

@tisdall
Copy link

tisdall commented Sep 7, 2017

@blacktaxi - I have a very strange issue with CKE 4 where Grammarly seems to be appending the first word it finds on the page to the end of the text editor. We have 2 text editors on the page and it appends it to the end of both the word "Schedule" which is the first menu item at the top of the page (basically, the first text item in the html body).

@tisdall
Copy link

tisdall commented Sep 7, 2017

@blacktaxi - I just got permission to share this video of the issue: https://youtu.be/GUBbycBDPOY?t=44s

EDIT: watching the video again, I noticed a small third-party added button that says "Schedule" and I think that may be what the issue is in this specific case (not Grammarly).

@Reinmar
Copy link
Member

Reinmar commented Sep 8, 2017

@tisdall I know that CKEditor 4 was mentioned in this thread, but we decided to focus here on CKEditor 5 (which is a completely new project). If you have issues with Grammarly and CKEditor 4 please either report them directly to the Grammarly team or on https://github.com/ckeditor/ckeditor-dev.

Thanks!

@wwalc
Copy link
Member

wwalc commented Feb 22, 2019

Hi @blacktaxi, do you plan to work on better integration with CKEditor 5 or in general with modern rich text editors or is it still unplanned? We've got customers and prospects looking for such feature from time to time and we're completely blocked as this is something that we cannot fix on our end.

@blacktaxi
Copy link

Hi @wwalc, thanks for your interest. We're currently working on RTE support. We have already released support for Draft, Quill, Prosemirror on limited set of sites. The general release is under way, but I can't give an ETA right now. We will be looking into supporting CKE as well.

@wwalc
Copy link
Member

wwalc commented Feb 22, 2019

@blacktaxi that sounds promising, thank you for the update. If you need any help from our side, please feel free to ping us.

@slotterbackW
Copy link

slotterbackW commented May 7, 2021

Just adding to this issue that Grammarly seems to work both on our implementation of CKEditor and on the CKEditor demo page. We've noticed some strange behavior in Firefox, but can't consistently reproduce, so that may be an unrelated issue.

One "gotcha" we ran into is that Grammarly requires the height of your editable element to be at least 38px (https://support.grammarly.com/hc/en-us/articles/115000091592-How-does-Grammarly-s-browser-extension-work-), but once we fixed that everything seems to work 👍🏻

@Mgsy
Copy link
Member

Mgsy commented May 17, 2021

Indeed, I can confirm that sometimes Grammarly in Firefox might behave unstable, especially as it comes to nested editables like table cells, image captions.

@blacktaxi, if there's something we can do to help you with improving the behaviour in Firefox, don't hesitate to ping us, we'll be happy to help.

@dufipl
Copy link
Contributor

dufipl commented Jan 17, 2022

Update 17.01.2022
During some testing of how Grammarly behave when typing in CKEditor 5, I've encountered a new issue: grammar checking is not working when text is put into the blockquote.
Here is an example how it looks like:
no grammar check in blockquote

This means that grammar checking is not working correctly anymore and thus I'm reopening the ticket.
If I find another issue with Grammarly behaviour in CKEditor 5, I will update in the comments.

@Mgsy Mgsy reopened this Jan 17, 2022
@Mgsy Mgsy changed the title External manipulation of the DOM (Grammarly) Integration with Grammarly Jan 17, 2022
@octavianSandu
Copy link

How was this eventually handled?

Did you make changes so that Grammarly has a way to modify the content?
Or did Grammarly devs found a way to get around the model checks?

@CKEditorBot
Copy link
Collaborator

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

@CKEditorBot
Copy link
Collaborator

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).

@CKEditorBot CKEditorBot added the resolution:expired This issue was closed due to lack of feedback. label Nov 2, 2023
@CKEditorBot CKEditorBot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution:expired This issue was closed due to lack of feedback. status:discussion status:stale type:feature This issue reports a feature request (an idea for a new functionality or a missing option). type:question This issue asks a question (how to...).
Projects
None yet
Development

No branches or pull requests