Fix: Incomplete CSS margin parsing if "auto" or "0" without unit is used

[hub33k]

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?

* [#1330>](https://github.com/ckeditor/ckeditor4/issues/1330): Fix converting margin style shorthand to separate styles.

What changes did you make?

Changed regex for splitMarginShorthand function so it now converts margin shorthands correctly.

Which issues does your PR resolve?

Closes #1330.

[f1ames]

I'm not sure if we need manual test here since it doesn't bring any additional value over unit tests. We just need to check input - output of this method. And since it is more an integration test than unit one (checking entire flow using editor instance) we have (almost) exactly the same execution flow as in manual test.

[f1ames]

Looks good 👍 Unit tests are now where they should be too.

I only had some doubts related to used regex (both versions in fact), please see my comment.

[hub33k]

Rebased onto latest master.

[hub33k]

Red CI is caused by #4285.

[f1ames]

Rebased onto latest master.

[f1ames]

I added two unit tests checking backwards compatibility for handling invalid (or partially invalid) values. It would be good to keep this behavior consistent since they may be some editor parts (hopefully not) which relays on this behavior. It turned out that || [ '0px' ] alternative (mentioned somewhere in #4270 (comment)) is still needed 🙈

Now it handles invalid values the same way it did before, but some tests fails on IEs. It would be good to check what's going on. Since also invalid values tests fails there it might be good to check how it behaved with the previous regexp version there.

IE8:

IE9/IE10/IE11:

[hub33k]

One test is failing (checked on Chrome)

Now it works (had to clean cache).

[hub33k]

It seems that IE completely removes any invalid styles that occurs in code 🤔

rem is  invalid in IE8, becouse it's supported from >= IE9 https://caniuse.com/rem

@f1ames, should we omit testing invalid values in IE?

[f1ames]

It seems that IE completely removes any invalid styles that occurs in code

Yes, if IE just doesn't accept invalid values we may skip those tests 👍 Btw. have you checked if it works the same with the previous regexp (if it's a browser fault it should).

rem is  invalid in IE8, becouse it's supported from >= IE9 https://caniuse.com/rem

I assume it is also removed in IE8 as invalid value. Is there any unit we can use here instead of rem (apart from px and %) to work also on IE8? I guess pt would be sufficient for example?

[f1ames]

See #4270 (comment).

[hub33k]

With previous regexp on IE8 these tests fails:

With previous regexp on IE11 these tests fails:

[hub33k]

I guess pt would be sufficient for example?

Yes, it works on IE8. I assume we want to change rem to pt?

[hub33k]

@f1ames I've replaced `rem` with pt and ignored testing invalid values on IE.

[f1ames]

LGTM 👍