[PFW] Improve integration with Color Button plugin

[Marek Lewandowski]

New PFW implementation puts font background as span with background css property. It's not handled by any of our features (colorbutton in particular), and therefore ACF strips it in the process.

If we'd set it as span background-color everything would work OK.

Additionally we should respect ​`config.colorButton_backStyle`, that way it will always use a proper style.

1. Open ​PFW full manual test case.
2. Open any document with font background in Word (e.g. last two lines in ckeditor-dev/tests/plugins/pastefromword/generated/_fixtures/Fonts/Fonts.docx).
3. Select some text with background in Word.
4. Copy it.
5. Paste into the manual test editor.

Expected: Pasted rich text preserves text background.

Actual: Background gets removed.

Reproduced on Chrome @ Win10.

[kkrzton]

I can confirm it also happens on FF.

[Marek Lewandowski]

So the source of the problem was that Word is pasting text background as e.g. background:yellow. When adding a colorbutton plugin it adds background-color to the ACF but not background.

There were two possibilities to fix it:

• Adding an additional handling to the Paste from Word plugin - converting background property to background-color if possible.
• Adding a more generic fix in the colorbutton plugin itself. If span's background contains only color then it make sense to convert it to background-color so that colorbutton plugin can handle it.

I went with the second option as it will enhance more cases than Paste from Word only. It's important to highlight that this transformation can only happen if color is the only property specified by the background property, in any other case we should abort transformation and leave it untouched.

This feature is controlled with a config option in case someone would like to disable it, with a config.colorButton_normalizeBackground and it defaults to true.

With that I've added two namespaces in CKEDITOR.tools that I've been always thinking about, first is CKEDITOR.tools.array for all the array-related methods and the other is CKEDITOR.tools.style for any style related stuff. It's just the beginning, and there are just a few methods. We'll put more functions with a follow-up ticket, also we will deprecate moved methods directly in CKEDITOR.tools, but that's out of scope of this ticket.

Pushed to ​t/16624.

[Jakub Ś]

#16602 was marked as duplicate.

[kkrzton]

The second approach indeed seems better IMHO.

I was only wondering if this fix should be tied to colorbutton plugin. Without the plugin it won't work. However, the colorbutton plugin is the one adding background ACF rule so it seems like a proper place for the fix. The only case in which it will not work is when the same rule is added manually (without colorbutton plugin), but then we should assume that other ACF rules fixing the issue also should be add manually.

The solution looks pretty solid, just a few things to check/fix:

• Accoridng to specification on MDN, the ​hue value in hsla can be a number but used ​regexp handles only integers.

• The CKEDITOR.tools.style.background returns only first color. This is ok and probably intended, however, ​the method description is a little misleading. Return Parsed background values as an array - as every color is a separate value, one may expect that it will return all parsed colors.

• For the array tools we could document what parameters will be ​passed to the callback function. While it uses same parameters as native implementation I am not sure if it is really needed.

[Marek Lewandowski]

Replying to k.krzton:

I was only wondering if this fix should be tied to colorbutton plugin. Without the plugin it won't work. However, the colorbutton plugin is the one adding background ACF rule so it seems like a proper place for the fix.

It make sense for colorbutton plugin as this is the plugin that provides background-related feature. Also it's colorbuttons implementation detail that it requires background-color rather than just background.

The only case in which it will not work is when the same rule is added manually (without colorbutton plugin), but then we should assume that other ACF rules fixing the issue also should be add manually.

Could you elaborate the example above? I'm not sure if I got it right.

• Accoridng to specification on MDN, the ​hue value in hsla can be a number but used ​regexp handles only integers.

Thanks, fixed in ​git:b24993d.

• The CKEDITOR.tools.style.background returns only first color. This is ok and probably intended, however, ​the method description is a little misleading. Return Parsed background values as an array - as every color is a separate value, one may expect that it will return all parsed colors.

Absolutely, I did not reflect changes made with ​git:e3cf0c9d73e4725ee9ba5bfe9475b0277ae683e8 in the API docs. I made a more "fun" but also more complex initial implementation, but in the end the benefit did not justify the complexity.

Initially it supported props like: background: red, blue url(foo.png) repeat-x, url(bar.png) top right and it would return an array with 3 objects { color: 'xyz', unprocessed: 'foo' }.

As for the current implementation, returingin a single color is intended just like you noted. Look at browser behavior, those will use only first color, and ignore any others. So we pick first color to color property, and remove any other from unprocessed not to pollute it with garbage.

• For the array tools we could document what parameters will be ​passed to the callback function. While it uses same parameters as native implementation I am not sure if it is really needed.

Make sense, it will be more readable, fixed in ​git:2ff808d.

Pushed changes back to ​t/16624.

[kkrzton]

LGTM!

---

The only case in which it will not work is when the same rule is added manually (without colorbutton plugin), but then we should assume that other ACF rules fixing the issue also should be add manually.

What I meant here is that you can add ACF rule to allow background-color property to be preserved while pasting (without including colorbutton plugin). Then you may expect that copying text with background color from Word will preserved the color while this rule is present. However, it won't because pasted content will have background property which will be removed.

But I also assumed here that if someone manually sets ACF config like this, he may easily adjust it also to preserve background property.

[kkrzton]

Merged to master with ​22b22b3.

[Marek Lewandowski]

Clarified description.