Preserve repeated spaces when text is wrapped with a CSS white-space rule

[jessevanassen]

Suggested merge commit message (convention)

Fix (engine): Preserve repeated spaces in text that is contained within an element that renders (repeated) white space. Closes #16124

---

Additional information

This pull request offers a possible solution for #16124.

It will preserve (repeated) whitespace when the text is contained within a block that preserves whitespace during rendering, which is done through the white-space CSS property. This is similar to what the code for the <pre> element is already doing.

Existing code within CKEditor is already taking care of swapping out the repeated spaces by  -patterns, so they are also preserved when the data is extracted again.

[Dumluregn]

Hi, thank you for contributing to our repository.

1. To accept your code, we need you to sign the Contributor License Agreement (CLA) - see https://ckeditor.com/docs/ckeditor5/latest/framework/contributing/contributing.html#contributor-license-agreement-cla.
2. Since the problem you're fixing isn't reproducible in the editor itself, please provide a sample environment showing that the fix works.
3. What if the white-space property is set in the style sheet instead of directly in data?
4. There are already two custom matchers in the DomConverter: https://github.com/jessevanassen/ckeditor5/blob/54ebaddc4f3a077e479fa636fd3675a51eb5400b/packages/ckeditor5-engine/src/view/domconverter.ts#L143-L148. Maybe it would be possible to implement your check in a similar manner? It could make checking the ancestors more difficult probably though, so it's not a must.

[jessevanassen]

Hi @Dumluregn, thanks for coming back to me!

1. I've already signed the CLA
2. The sample environment is quite easy: just an HTML page containing the CSS rule applied globally, and some content with some white space padding around it. I put a sample in the description of Repeated spaces are not preserved when text is contained in an element that renders whitespace using a CSS white-space rule. #16124.
   Would you like me to add this test case to the test/manual folder of the repo as well?
3. This depends on the browser. The Chromium / Webkit family does it differently than the Firefox browsers.
   In a Chromium or Webkit browser, the HTML that is placed on the clipboard is derived from what is rendered. So if for instance white-space: pre-wrap is part of a style sheet, the HTML nodes on the clipboard will still get a style="white-space: pre-wrap" attribute.
   In Firefox on the other hand, it will basically copy over what is in the DOM, without adding additional style attributes. This means that in Firefox the white-space will only be included if it is part of the copied content, or if the copied content is wrapped in an inline element containing a style="white-space: pre-wrap" attribute (which is the hack we're currently using in our application to force the attribute on the element).
4. I will take a look at that later and come back to you, thanks for the pointer!

[jessevanassen]

I've looked at the Matchers, but to me it feels like they wouldn't be the right approach for this check.

If I understand them correctly, a Matcher is something that can be initialized with different kinds of patterns, and can then be used to "filter" elements.

However, for the check, I want to see if a specific element is preformatted, and take specific actions if it is (or isn't). I could try to implement it with a Matcher, the pattern that is a function that takes an Element can probably be used for this. I think turning it into a Matcher will make it less convenient to use though, without any benefits. The result of the Matcher (which element, rule, attributes or styles matched, etc.) will be irrelevant, so in the calling code I would only be checking if I have a match, but the actual details won't be used. And the Matcher also requires more setup, and isn't immutable after being constructed (more patterns can be added later which isn't desirable).

Like it is now, a pure function that returns a boolean, feels better suited for the task. Would you agree, or do you have other insights?

[Dumluregn]

I see it is indeed simple to reproduce the problem in any manual test, so I'll just repost it here:

It's also really easy to reproduce in one of CKEditor's manual tests, by setting the editor data to something like
<span style="white-space: pre-wrap">Hello, World!</span>

Thank you for investigating the Matcher approach. I think it brings too little benefits to pursue it.

Could you please merge the latest master branch and ensure it still works fine? I left just one comment and we'll be good to merge this PR.

[jessevanassen]

I've made it up to date with master, and all the tests still pass

[Dumluregn]

LGTM. Thank you for the contribution!