Fix fire print on Firefox

[KarolDawidziuk]

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

• Unit tests
• Manual tests

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.

• PR is consistent with the code style guide

What is the proposed changelog entry for this pull request?

* [#<4790>](https://github.com/ckeditor/ckeditor4/issues/4790): Fixed: Incorrect fire Print on FF

What changes did you make?

The problem was with the page load status in FF mode, because an attempt to print was triggered when the preview page was not ready. So I added a onreadystatechange callback to wait for the complete status.
Also, due to the rollback of my previous changes related to the CDN opening "preview" problem #4444 I am adding these changes to this PR.

Which issues does your PR resolve?

Closes #4790.
#4444.
#3876

[Comandeer]

I'm thinking about refactoring the whole thing a little bit and moving most of its part into preview plugin (see my inline comments).

Additionally there are two failing tests inside print suite (the one you modified and one about immediately invoking print on document.readyState === 'complete'). However, instead of fixing, we should probably remove them and instead add some tests to preview plugin to check if passed calback has been executed in the right time.

[KarolDawidziuk]

Thanks, @Comandeer for refactoring and review.
It's ready for another one.

Changes:

• update docs,
• revert old version of unit test from tests/../print.js,
• removed two unit tests from tests/../print.js,
• added unit test.

[Comandeer]

Unfortunately our current approach does not work correctly in IE as it does not change document.readyState to complete if there are any images in the content. Additionally simply switching to load event instead also won't work, because Safari does not fire load for subsequent opening of preview window. It means that we have to introduce two different flows – one for IE, using (probably) load event and other one for all other browsers.

[Comandeer]

Blocked by #4824.

[KarolDawidziuk]

@Comandeer, I've added two ways to invoke the print callback. One for IE with using onload and onreadystatechange for all other browsers.

[Comandeer]

Overally looks good, there's only one issue with the moment of invoking callback in IE (see inline comment).

Please also rebase the PR onto latest master.

[KarolDawidziuk]

Thanks, @Comandeer for the review.

Now it's working properly for me on every browser, and it's ready for another review.
Also, due to the rollback of my previous changes related to the CDN opening "preview" problem #4444 I am adding these changes to this PR.

[Comandeer]

Fix seems to work well, good job!

However there are still some small issues:

• see my inline comments;
• there is a mismatch between manual test .md and .html file, resulting in non-functional test – please fix it;
• it'd be good to add two more unit tests – one for IE and checking if callback is really fired on load and one for other browsers to check if it's fired on readystatechange with appropriate readyState;
• and traditionally – rebase onto latest master.

[KarolDawidziuk]

Thank's @Comandeer for the review.

I added all the things you mentioned except the unit tests because we must using sinon.stub to prevent the new window from opening due to a failed test and adding onload and onreadystatechange is impossible in this case.

[Comandeer]

After adding #3876 to issues closed by this PR, we need some manual test for it. Fortunately there is already one, /test/plugins/print/manual/print – just add version and issue tags and it's good to go.

There is also a small issue with previewcdn manual test. At the moment it's failing in Firefox due to the fact that it uses CKEditor 4 from the latest master branch, so without the fix. I don't know if there's sensible way to fix it 🤔

[Comandeer]

Suggested change

	if ( bender.env.mobile ) {
	if ( bender.tools.env.mobile ) {

[KarolDawidziuk]

Thanks @Comandeer for the review.
It's ready for another one.

Changes:

• Added proper changes in the print manual test
• Fix skipping test on mobile.

With the earlier PR fixing #4444 problem, we decided to use the master version, because when the logic of plugin changes, we won't have to remember to update this test. Additionally, we are sure that the simulated editor will use the latest version.

[Comandeer]

LGTM! 🎉 I've just renamed fireCallback and printCallback to previewCallback.