-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Prevent invocation on not existing editable [FF] #4800
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix itself looks reasonable and works fine 👍 I have only some doubts regarding manual test (see review comment).
I wasn't able to get this error when I switched modes with automatic tests. So I decided to use the manual one.
👍
Hi, @f1ames - PR is ready for another review. I've removed the gecko restriction from manual test. The mobile restriction is still there due to the dev console usage... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there 😉 👍
Hi @f1ames, |
Rebased onto latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
What is the purpose of this pull request?
Bug fix
Does your PR contain necessary tests?
All patches that change the editor code must include tests. You can always read more
on PR testing,
how to set the testing environment and
how to create tests
in the official CKEditor documentation.
This PR contains
Did you follow the CKEditor 4 code style guide?
Your code should follow the guidelines from the CKEditor 4 code style guide which helps keep the entire codebase consistent.
What is the proposed changelog entry for this pull request?
What changes did you make?
I just added some prevention in one
if
statement. So in case the editable isnull
nothing happened. The source of the issue is described here. Also this event is fired again anyway after the editable is fully initialized. So ignoring it one time (actually only in FF) doesn't break the feature.I wasn't able to get this error when I switched modes with automatic tests. So I decided to use the manual one.
Which issues does your PR resolve?
Closes #4782 .