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

Fix for failing tests on Edge 16+ #1105

Closed
wants to merge 41 commits into from
Closed

Fix for failing tests on Edge 16+ #1105

wants to merge 41 commits into from

Conversation

msamsel
Copy link
Contributor

@msamsel msamsel commented Oct 31, 2017

What is the purpose of this pull request?

Bug fix

Does your PR contain necessary tests?

fix tests

This PR contains

  • Unit tests
  • Manual tests

What changes did you make?

  • split test to run in separate editors

Notes:
I've tried to find out where is the problem with this test, unfortunately without any clear result.
From some unknown reason resume method wasn't execute properly.
https://github.com/ckeditor/ckeditor-dev/blob/96bf4037cb3ff812fc9b993bce4a304a884f361a/tests/plugins/tableselection/integrations/clipboard/pasteflow.js#L34-L44
I've tried to delay it by some timeout, but without proper reaction.
I also noticed that, when Edge starts to fail, then selection in editor disappears (became collapsed). I test also possibility to change way how selection is made with switching bot.setHtmlWithSelection into bender.tools.selection.setWithHtml, but second method doesn't support multiple ranges.
https://github.com/ckeditor/ckeditor-dev/blob/96bf4037cb3ff812fc9b993bce4a304a884f361a/tests/plugins/tableselection/integrations/clipboard/pasteflow.js#L27

So to prevent of possible future fails, I've redesign test to run in separate editors. What fix the issue.

Blocked by: #962
Close: #1047

@mlewand
Copy link
Contributor

mlewand commented Nov 10, 2017

I believe this PR was closed due to my merge messup in #968 (comment)

So we'll have to create a new PR and cherry-pick @msamsel commit.

@f1ames f1ames mentioned this pull request Nov 13, 2017
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants