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

[Chrome] Error throws in the image description after random typing #578

Closed
ma2ciek opened this issue Oct 2, 2017 · 12 comments
Closed

[Chrome] Error throws in the image description after random typing #578

ma2ciek opened this issue Oct 2, 2017 · 12 comments
Assignees
Labels
type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@ma2ciek
Copy link
Contributor

ma2ciek commented Oct 2, 2017

Steps to reproduce

  1. Go to https://ckeditor5.github.io/docs/nightly/ckeditor5/latest/features/image.html or https://ckeditor5.github.io/
  2. Type randomly in the image description

Error:
screen shot 2017-10-02 at 12 44 24

@ma2ciek ma2ciek added the type:bug This issue reports a buggy (incorrect) behavior. label Oct 2, 2017
@Reinmar
Copy link
Member

Reinmar commented Oct 2, 2017

Hm... confirmed. When I type blabla quick enough the error is thrown.

@Reinmar Reinmar added this to the iteration 12 milestone Oct 2, 2017
@Reinmar
Copy link
Member

Reinmar commented Oct 2, 2017

I can't reproduce this issue on http:https://localhost:8125/ckeditor5-image/tests/manual/caption.html though. And I can do this on http:https://localhost/ckeditor5/build/docs/ckeditor5/0.11.0/examples/builds/classic-editor.html. Interesting. It seems that page styling affects it.

@Reinmar
Copy link
Member

Reinmar commented Oct 2, 2017

Yeah, that's definitely due to styling because when I loaded the docs before Umberto generated their CSS, I could not reproduce this issue.

@ma2ciek
Copy link
Contributor Author

ma2ciek commented Oct 2, 2017

It's interesting that styling might easily destroy the editor.

@Reinmar
Copy link
Member

Reinmar commented Oct 2, 2017

OK, for some reason, a common ancestor of these mutations is a text node. Such situation should not be handled by _handleContainerChildrenMutations(). I wonder what's inside the mutations array.

@Reinmar
Copy link
Member

Reinmar commented Oct 2, 2017

OK, it turns out that this is Grammarly:

image

It seems that in #267 we missed that Grammarly kicks in in nested editables.

Before we'll ping Grammarly's team, we need to figure out whether we can't just add .ck-editor__editable to nested editables (which, I guess, would disable Grammarly).

cc @oleq

@Reinmar Reinmar modified the milestones: iteration 12, iteration 13 Oct 2, 2017
@oleq
Copy link
Member

oleq commented Oct 3, 2017

We could add this class but it will require some refactoring of the styles across the project. There are many usages of the .ck-editor__editable in various stylesheets and we must understand their purpose, then rewrite the styles to consider the more generic role of the class.

@Reinmar
Copy link
Member

Reinmar commented Oct 13, 2017

@oleq, could you estimate how hard would this change be? And if it makes sense in general? I see that right now, nested editable has only the .ck-editable class... which is odd. Unification sound like a thing we should do.

@oleq
Copy link
Member

oleq commented Oct 13, 2017

The change is not hard but it is spread across multiple repositories and, most certainly, Letters too. I think the right moment to do this will be after #420, though.

@Reinmar Reinmar modified the milestones: iteration 13, iteration 14 Nov 14, 2017
@Reinmar Reinmar modified the milestones: iteration 14, iteration 15 Jan 25, 2018
@oleq
Copy link
Member

oleq commented Mar 20, 2018

I cannot reproduce this issue. Can anyone confirm?

@Reinmar
Copy link
Member

Reinmar commented Mar 20, 2018

Not this one, but e.g. this:

image

Grammarly inserts bazillion text nodes with spaces into the editor when you type some random letters quickly. It doesn't do that when it's completely disabled.

@oleq
Copy link
Member

oleq commented Mar 20, 2018

OK. I check that and the .ck-editor__editable class fixes the problem. I'll prepare a multi-repo PR.

szymonkups added a commit to ckeditor/ckeditor5-widget that referenced this issue Mar 30, 2018
Fix: Replaced nested editable's `.ck-editable` class with `.ck-editor__editable` + `.ck-editor__nested-editable` to stop Grammarly throwing errors. Closes ckeditor/ckeditor5#578.

BREAKING CHANGE: The `.ck-editable` class is no longer available. Use the `.ck-editor__nested-editable` class instead.
szymonkups added a commit to ckeditor/ckeditor5-image that referenced this issue Mar 30, 2018
Tests: Updated image tests to the latest nested editable CSS class naming convention (see ckeditor/ckeditor5#578).
szymonkups added a commit to ckeditor/ckeditor5-theme-lark that referenced this issue Mar 30, 2018
Other: Updated the classic editor and the editorui styles to the latest nested editable CSS class naming convention (see ckeditor/ckeditor5#578).
szymonkups added a commit that referenced this issue Mar 30, 2018
Docs: Updated snippet styles to the latest nested editable CSS class naming convention (see #578).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

3 participants