Replace "alert()" with warning notification

[Piotr Jasiun]

Since we have notifications we can replace some usage of alert() method with warning notification. I found 3 potential places where we could change it:

• ​https://github.com/ckeditor/ckeditor-dev/blob/major/plugins/clipboard/plugin.js#L640
• ​https://github.com/ckeditor/ckeditor-dev/blob/major/plugins/pastefromword/filter/default.js#L1162
• ​https://github.com/ckeditor/ckeditor-dev/blob/major/plugins/filebrowser/plugin.js#L342

Warning notification could be used if notification plugin is loaded and alert function otherwise.

[Frederico Caldeira Knabben]

There is no reason to assign this to the 4.5 though.

[Piotr Jasiun]

This ticket occurred to be interesting test of the notification API.

First of all it seems to be a good solution to have a method in the editor which show an alert, if there is no notification plugin, and show notification, if there is. Therefore plugins which want just show basic information does not need to require notifications plugin. Editor should know nothing about plugins, so that method should not check if notification plugin is available, but because notification plugin adds editor.showNotification, editor can have default showNotification method which use alert.

Second problem was what kind of notification should be default. My fist approach was to change the default type to warning. It is more similar to alert, needs more attention, does not hide automatically. Also messages used in alert are called error*. But then I realized that we used alert only because we did not have better notification for these cases and they do not need that much attention and are the prefect example for the info notification (when user press "Cut" or "Copy" button on Chrome or Firefox, he should see information, not warning). So I switched back to info as a default notification type.

It turned out that file browser use alert in the dialog, so I left alert there and checked other two plugins.

I pushed changes and manual test to ​t/12870b. In ​t/12870 branch you can find warning messages as default, but I believe they looks worse.

Because it is part of the API changes it should goes to 4.5.

[Piotrek Koszuliński]

I pushed rebased ​branch:t/12870b with an additional commit.

R- because editor.showNotification lacks automated test. Please also remove the manual test which checks a thing that can be checked by an automated test. Don't create manual tests for things that can be so well tested automatically.

[Piotr Jasiun]

I have added some unit tests for showNotification to check if it call alert if there is no notification plugin and if it is called in case of errors.

I find manual test useful. Thanks to this test I realized how this alert work in the context and decided to change the notification back to info, from warning. This is a UI feature which can not be checked only by unit tests. Also this manual test show the new feature and since during release developers are not checking tickets anymore (thanks to manual tests), this is good way to show that some editors behavior changed.

[Piotrek Koszuliński]

Merged to major with ​git:e112a76.