WidgetBuffer creates wrap opportunity in chrome

Question

WidgetBuffer creates wrap opportunity in chrome

mrdrogdrog opened this issue 2 years ago · 19 comments

Describe the issue

Hello,

We're using CodeMirror 6 in the development of HedgeDoc 2. So far I'm very impressed by what you've done so far. Great piece of work ❤️

Anyway. There is one problem. As a collaborative editor we want to show the remote cursors. For this task we use a widget (hedgedoc/hedgedoc#2957 to be precise), which causes trouble in combination with EditorView.lineWrapping. To be precise: Chrome takes the widgetBuffer as wrap opportunity because it is an img tag.

hedgedoc-2022-11-04_22.03.14.mp4

It seems like this commit is causing the issue. While I understand that this solution is maybe solving some other problems, it's causing this one which makes it unusable for us. ☹️

Quick solution idea

What about letting the widget itself choose if the buffer should be an img or a span?

Additional Context

hedgedoc/hedgedoc#2957
hedgedoc/react-client#2451

Browser and platform

Chrome

Reproduction link

No response

Answer 1 · 2022-11-12T14:11:13.000Z

I have a reproduction link without hedgedoc or remote cursors.

As you can see, the line is split after the <img class=cm-widgetBuffer> even though the line content is only a single long word.

Answer 2 · 2022-11-12T15:14:43.000Z

I am aware of this, but haven't been able to find a good workaround. The zero-width space thing we had before caused a lot of trouble because, after the browser does its thing on some native edit action, it was had to distinguish the spaces that were inserted for this hack from spaces that should actually be in the document. Also I'm pretty sure (and a rough test confirms) that zero-width spaces are also used as wrap opportunities by browsers—are you sure the patch you linked made a difference here?

What we need, basically, is a bit of DOM that is A) treated as a regular editable inline element by the browser so that it avoids the slew of browser bugs around having a cursor next to an uneditable element, B) is invisible, C) is easy to distinguish from content text, and D) does not create a wrap point. If you find some obscure element or style that lets us do this, do let me know!

Answer 3 · 2022-11-12T15:19:59.000Z

(Possibly unicode character 0x2060, "word joiner" can be useful here. But again, we will need to support such characters appearing in the document as well, and somehow tell leftovers from our widget buffers apart from inserted content.)

Answer 4 · 2022-11-12T16:22:13.000Z

As far as you've described you just need a dummy element whose only purpose is to exist and don't influence anything, right?
Have you tried custom tags ?
You don't need any logic or lifecycle behind them for this purpose. Just place an element mit a custom name in the dom like... <codemirror-widgetbuffer/>. It's valid HTML5 because of the dash inside and if you don't define any logic then the browser should ignore it in the rendering/layouting step and it shouldn't create a wrap opportunity 🤔
It's just a quick idea. I couldn't test it myself so far.

Answer 5 · 2022-11-12T16:47:56.000Z

Have you tried custom tags ?

Those won't be treated as significant inline text editing elements by browsers (or a <span> would also work). So the cursor will still be regarded as directly next to the uneditable element even though such an element is between them.

Answer 6 · 2022-11-12T17:09:42.000Z

then what about an empty span?

edit: hmpf.. this is also likely to be ignored because you need actual content, right?

Answer 7 · 2022-11-12T17:15:42.000Z

yes.. that's really tricky o.ö

Answer 8 · 2022-11-13T07:25:01.000Z

Oh...

FYI, I have just found that chrome devs also think that elements with "position: absolute" style should not introduce soft wrap opportunities (though they have left the crbug open for years). So I think this could be, theoretically, solved by "position: absolute", if only the crbug were fixed... 😢

Answer 9 · 2022-11-15T09:12:44.000Z

Unfortunately even without the WidgetBuffer I think you'll probably find that any decoration which splits a word will cause a wrap opportunity.

The only way I know of to resolve this is to draw the collaborator cursors in a separate layer, as CodeMirror does for the main cursor.

Answer 10 · 2022-11-15T09:38:45.000Z

Actually I had this problem while working on remote cursors and solved that using nbsp inside of the widget.

Answer 11 · 2022-11-20T18:25:07.000Z

I just had another idea (thanks bathtub 🛀). How about wrapping the whole text before and after the widget buffers into editable spans and let only them do the text wrapping?

In the following example you would only allow wrapping on elements with class text.

<span class="not-wrappable">
<span class="wrappable">i'm a text</span><img>[...]<img><span class="wrappable"> after the widget</span>
</span>

Answer 12 · 2022-11-22T12:08:39.000Z

Unfortunately even without the WidgetBuffer I think you'll probably find that any decoration which splits a word will cause a wrap opportunity.

This is a good point. This patch exports the logic used by drawSelection for external code, and may be a better fit for drawing additional cursors than widget decorations.

Could you take a look and let me know if this feature would work for you?

Answer 13 · 2022-11-22T12:20:59.000Z

This looks interesting. I'll take a look later.

Answer 14 · 2022-11-26T10:19:50.000Z

This is a good point. This patch exports the logic used by drawSelection for external code, and may be a better fit for drawing additional cursors than widget decorations.

Could you take a look and let me know if this feature would work for you?

Looks like it is working! Thank you!
https://github.com/tamo/y-codemirror.next/blob/ce2bfbd8447a1c41727a09350d141733f9ca9714/src/y-remote-selections.js

Here are a couple of ideas I got while writing it:

Maybe a hook in LayerView.destroy would be useful.
I set a listener in mount() but the listener will never be off.
measureRange may be useful if exported.
I would have used measureCursor and measureRange if they had been exported.
I just copied measureCursor and commented out selection markers.

Answer 15 · 2022-11-30T12:29:26.000Z

@tamo Could you take a look and see if attached patches would cover your requirements?

Answer 16 · 2022-11-30T12:30:30.000Z

I'm going to close this issue as wontfix, since adding content inline necessarily influences break points, and it seems really hard to control that in a way that doesn't cause more problems than it solves.

Answer 17 · 2022-12-06T22:15:40.000Z

Just to let you know: The layer method works fine so far. We use it right now to create a new cursor plugin that fits our use case. Thank you for your help ❤️
But it would be nice if you would create a release of codemirror/view with the exported measurement functions. :)

Answer 18 · 2022-12-07T08:39:35.000Z

I've tagged @codemirror/view 6.7.0

Answer 19 · 2022-12-11T10:37:10.000Z

Thank you all, I'm happy to know that hedgedoc works well now.

@tamo Could you take a look and see if attached patches would cover your requirements?

Thanks for the patch. Probably it's good enough.
I tried to use them in @yjs /y-codemirror.next but I failed because I was not skilled enough.
Maybe the yjs dev will adopt the layers functionality.