Paste From Word settings are reversed

[Tade0]

Steps to reproduce

1. Set pasteFromWordRemoveFontStyles to false.
2. Paste content from Word with font size, color etc.
3. Set pasteFromWordRemoveFontStyles to true.

Expected result

Styles are removed when the setting is true.

Actual result

Styles are removed when the setting is false.

Other details (browser, OS, CKEditor version, installed plugins)

[kkrzton]

Seems like an error/typo in the code.

While previously the config.pasteFromWordRemoveFontStylesoption was used like

removeFontStyles = config.pasteFromWordRemoveFontStyles !== false

(​plugins/pastefromword/filter/default.js#L684) in the new version of PFW it is used like

removeFontStyles = config.pasteFromWordRemoveFontStyles === false

(​plugins/pastefromword/filter/default.js#L417). It appearsed in a new version of PFW to provide backward compatibility (​brought back here) so it should work the same way as previously.

---

The important thing here is that the default value for this config option had changed from false to true so the above seems like a proper change.

[kkrzton]

This one is tricky. It seems like the correct interpretation of config.pasteFromWordRemoveFontStyles is

preserveFontStyles = editor && editor.config.pasteFromWordRemoveFontStyles === false; or

removeFontStyles = !( editor && editor.config.pasteFromWordRemoveFontStyles === false );.

The value defaults to true so only when explicitly set to false, font styles should not be removed. Applying such fix shows the correct behaviour in a manual test but results in more than 50% of PFW unit tests failing (even after reversing config.pasteFromWordRemoveFontStyles value for test when it was used).

[kkrzton]

When setting config.pasteFromWordRemoveFontStyles = false for all tests (except the one which earlier had config.pasteFromWordRemoveFontStyles = false set explicitly), there are only 13 unit tests failing. However, I suppose we should use default value (which is true) for most of the tests.

[kkrzton]

Pushed changes to ​t/16653.

To sum up (previous comments may be somehow confusing, stating opposite things to each other as I created them ad hoc):

The pasteFromWordRemoveFontStyles option was in fact reversed and code needed to be updated as it assumed reversed flow (​46bff8b).

While this option had been reversed, the config for all tests have to be reversed as well (​873fbb8).

However, there are few things I am not sure about:

The code states that Prior to version 4.6.0 this configuration option defaulted to 'false'. According to the commit ​acb029e and SDK (​http://docs.ckeditor.com/#!/api/CKEDITOR.config-cfg-pasteFromWordRemoveFontStyles) it seems it defaulted to true. Also looking at the docs it seems like now it should have default value false (​plugins/pastefromword/filter/default.js#L1653).

As I updated this config value, most of the test had to be updated with pasteFromWordRemoveFontStyles: false option. I assume most of the cases should be tested with default value (as it is the most widely used also).

---

I see the intention might be to set default value to false (as opposite to true earlier). I will just recheck it and provide another fix branch.

[kkrzton]

Pushed changes to ​t/16653b.

Assumed that the default pasteFromWordRemoveFontStyles is false (opposite to t/16653). It required very little changes in the existing code. I will just to make sure what default value it should be.

[kkrzton]

The correct branch for R is ​t/16653b.

[Marek Lewandowski]

LGTM! Fixed in ​git:68643fdfb6680e95f2b40788754e3fc5d1e9eca0, merged to major.