A period does not stick to the preceding word during word-wrap

[neongreen]

📝 Provide detailed reproduction steps (if any)

1. Open https://ckeditor.com/ckeditor-5/demo/
2. Make sure you have one line that takes the full editor width and has a link at the end
3. Type a period . after the link

✔️ Expected result

The link and the period are word-wrapped

❌ Actual result

The period is detached from the word; if I press Enter after that, everything re-wraps normally

Screen.Recording.2021-01-17.at.21.38.02.mov

📃 Other details

• Browser: Chrome
• OS: macOS Catalina
• First affected CKEditor version: ?
• Installed CKEditor plugins: ?

---

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

[neongreen]

Is also reproducible in Safari.

[Mgsy]

Hi, thanks for the report. I can confirm this behaviour.

[oleq]

This bug is related to the inline filler logic and the filler sequence appearing in the text node

[oleq]

It could be this method is too permissive

ckeditor5/packages/ckeditor5-engine/src/view/renderer.js

Lines 408 to 445 in 4289176

	_needsInlineFillerAtSelection() {
	if ( this.selection.rangeCount != 1 || !this.selection.isCollapsed ) {
	return false;
	}
	const selectionPosition = this.selection.getFirstPosition();
	const selectionParent = selectionPosition.parent;
	const selectionOffset = selectionPosition.offset;
	// If there is no DOM root we do not care about fillers.
	if ( !this.domConverter.mapViewToDom( selectionParent.root ) ) {
	return false;
	}
	if ( !( selectionParent.is( 'element' ) ) ) {
	return false;
	}
	// Prevent adding inline filler inside elements with contenteditable=false.
	// https://github.com/ckeditor/ckeditor5-engine/issues/1170
	if ( !isEditable( selectionParent ) ) {
	return false;
	}
	// We have block filler, we do not need inline one.
	if ( selectionOffset === selectionParent.getFillerOffset() ) {
	return false;
	}
	const nodeBefore = selectionPosition.nodeBefore;
	const nodeAfter = selectionPosition.nodeAfter;
	if ( nodeBefore instanceof ViewText || nodeAfter instanceof ViewText ) {
	return false;
	}
	return true;
	}

[oleq]

I'm also not sure why the filler persists in nodes that don't need it. This will require some research. 

[niegowski]

I'm also not sure why the filler persists in nodes that don't need it. This will require some research.

It's to avoid breaking composition while the selection is inside the text node: https://github.com/ckeditor/ckeditor5/blob/master/packages/ckeditor5-engine/src/view/renderer.js#L343-L345

[oleq]

So this is a "have your cake and eat it" kind of situation? Either the composition works properly or the trailing punctuation behaves as it should?

Do you think there's a chance that moving to beforeinput will allow us to make this filler logic less intrusive?

[niegowski]

Maybe we could consider switching from "zero-width space" to "zero-width no-break space".

[niegowski]

I briefly checked it and replacing '\u200b' with '\ufeff' here

ckeditor5/packages/ckeditor5-engine/src/view/filler.js

Line 74 in b278fde

inlineFiller += '\u200b';

seems to do a trick.

[niegowski]

Seems that \ufeff is deprecated in the context of word joiner but it is not deprecated in general (https://en.wikipedia.org/wiki/Word_joiner). There is another Unicode character \u2060 (word joiner) that is supposed to replace the usage as word joiner but it has a side effect that it does not render in dev tools. I would still use \ufeff (zero-width non-breaking space) to avoid confusion while debugging in dev tools. WDYT @Reinmar ?

[Reinmar]

It's weird that a word joiner is not displayed in the dev tools. Could you file them a bug report? Their response may actually shed some light on this.

I laughed when I read that \ufeff is BOM :D I forgot about this gem. It makes mi slightly worried to use this character now because there may be some stupid BOM filters implemented in "enterprise" tooling. I've seen such things... But we'll never know if we don't try, so I think we could go with it. However, we'll need to pay attention during QA on whether this works in all browsers and OSes.

[niegowski]

I noticed that some tests in the typing package started to fail after changing ZWS with ZWNBSP because the ZWNBSP does match in regexps to \s:

/\s/.test( '\u200b' ) -> false (ZWS)
/\s/.test( '\ufeff' ) -> true (ZWNBSP)
/\s/.test( '\u2060' ) -> false (WJ)

So there could be many places where this could fail. Taking this and the BOM case I decided to go with WJ option and to report DevTools bug.

[niegowski]

I reported the issue for DevTools: https://bugs.chromium.org/p/chromium/issues/detail?id=1175133