TINY-10854: Now insert format also manage nbsps correctly

[lorenzo-pomili]

Related Ticket: TINY-10854

Description of Changes:
Since to normalize the nbsps we use normalizeNbsps but this mechanism is based on the text elements and to fix our case we would need to find the parent and parse all the children element which could require way more time.

Since when we add a format we apply a CaretFormat, so to fix this case my idea is to apply an nbsp normalization with on input (like for normalizeNbspsInEditor in InputKeys), but only for the next insertion (so using editor.once) since this is needed only after the first insertion.

Pre-checks:

• Changelog entry added
• Tests have been added (if applicable)
• Branch prefixed with feature/, hotfix/ or spike/

Review:

• Milestone set
• Docs ticket created (if applicable)

GitHub issues (if applicable):

[spocke]

• Fix typos in PR title npsp -> nbsp and correclty -> correctly

[TheSpyder]

I'm surprised that removing every nbsp passes the tests. But our tests for NBSP normalisation probably don't cover this new caretContainer condition well enough. Speaking of which...

Suggested change

	if (caretContainer && !editor.dom.isEmpty(caretContainer)) {
	if (caretContainer !== null && !editor.dom.isEmpty(caretContainer)) {

Try something like <p>text <strong>&nbsp;more text</strong></p>. Your code will probably remove the nbsp, which is wrong.

• do some wider checks of this logic

[lorenzo-pomili]

ok, I'm writing the logic for it, I'd like to make an abstraction starting from Nbsps.ts to use it in both cases, but I'm not sure how possible is since it's strong relation with the elements of the DOM.

Since the logic in Nbsps.ts isn't working for this cases, what logic do we want to implement?
by now we have these tests: NbspsTest.ts, should I make these cases work considering that instead of a simple text that could be an inline element?

[spocke]

I would break out the everything inside the isEmpty check. Then make a separate function that takes a target node that checks the sibling before target, first child of target, last child of target, sibling after target. Then checks if any of those nodes has a non whitespace character followed by a nbsp since then you can replace that nbsp with a space.
Maybe feed those siblings into an array if that makes things easier. We might need to expand this in the future for other cases like <strong>foo&nbsp;<span>bar</span> etc.
Maybe it makes sense to stick that new function into the Nbsps module?

[lorenzo-pomili]

I'm trying to figure out how this should behave, and at the moment in my mind is it should behave as if there were no inline element so:

inline after text
(single space) a b -> a <strong>b</strong> || a<strong> b</strong>
(double space) a  b -> a&nbsp; <strong>b</strong> || a&nbsp;<strong> b</strong> || a<strong>&nbsp; b</strong>

text after inline
(single space) a b -> <strong>a</strong> b || <strong>a </strong>b
(double space) a  b -> <strong>a</strong>&nbsp; b || <strong>a&nbsp;</strong> b || <strong>a&nbsp; </strong>b

now these examples not include things like "a\u00a0b" -> "a b" or your example @TheSpyder (<p>text <strong>&nbsp;more text</strong></p>) because I actually didn't find a way to reproduce this case typing :/

[TheSpyder]

Typing in TinyMCE won't reproduce all the edge cases because we do normalisation on space.

Browsers don't take inline tags into account when collapsing whitespace. If there are two spaces in a row, and one of them isn't an nbsp, it will render as one space.

[lorenzo-pomili]

at the moment the handler is triggered only on typing to not have performance issues, I'll try to check where the Nbsps.ts functions are used and see it is possible somehow distinguish if we need to also check for siblings

[Afraithe]

@TheSpyder that seems out of scope for this issue reported by client? The scenario outlined cause a great deal of &nbsp's if you typed in the certain way, that is the issue we want to fix. There can be other edge cases, but also cases where you might want nbsp's, we should be careful expanding on this to much imo.

[TheSpyder]

This change blindly removes all nbsp characters in the previous sibling of the cursor. I'm not the one who expanded the scope, I'm simply responding with potential issues given that new scope.

When I said this

Typing in TinyMCE won't reproduce all the edge cases because we do normalisation on space.

I didn't mean to imply the fix should run more often than when typing. Only that the scenarios it has to deal with include things that can't be created within the editor. Users copy and paste content from all sorts of places.

[lorenzo-pomili]

wait about the remove of all nbsp last version of the code shouldn't do it, for the paste part, the behavior is different between simple text and text with inline elements at the moment, I'll try to understand how to solve it

[spocke]

I would break out the everything inside the isEmpty check. Then make a separate function that takes a target node that checks the sibling before target, first child of target, last child of target, sibling after target. Then checks if any of those nodes has a non whitespace character followed by a nbsp since then you can replace that nbsp with a space.
Maybe feed those siblings into an array if that makes things easier. We might need to expand this in the future for other cases like <strong>foo&nbsp;<span>bar</span> etc.
Maybe it makes sense to stick that new function into the Nbsps module?