Wikimedia Bugzilla is closed!

Unicorning-related?

Yes, this is related to unicorning. There are several problems.

Link continuation is now no longer done through pawning, because pawning is evil and kills IMEs. However, now when you type at the end of a link, DM and CE get out of sync: DM knows to continue the link until the first word break, but CE will either continue indefinitely or not at all depending on the browser (in Chrome, it doesn't continue at all). This leads to interesting surprises: you type at the end of a link, the text you type doesn't appear linked, but as soon as you press backspace it does, but only the text up to the first space.

David and I had this idea of doing link continuation with unicorns. We could unicorn whenever the cursor is at the edge of a forceContinuation annotation. That would allow IMEs to type text there, and the text you type would either appear like it was all part of the link or like none of it was part of the link (looks like currently it's none of it) until such time as we can safely de-unicorn (generally when you move the cursor somewhere else).

This logic doesn't seem to be working very well. I can see it work in the standalone demo, but only when I move the cursor using the arrow keys (not when using the mouse) and only when moving from the right to the left (not when moving left to right). I think this might be due to some of Ed's selection changes causing the render lock to be engaged at the time when we would like to start unicorning. It doesn't unicorn at all in VE-MW, for reasons I don't understand.

Even if we do get this to work as originally designed again, the user experience isn't great. The typed text would appear as either linked or not linked, even past a word break, until the selection is moved. It would be nice if we could do one of the following:
* De-unicorn as soon as a wordbreak is inserted. I think it's likely that this isn't feasible because there are IMEs that can insert wordbreaks. David?
* De-unicorn aggressively, either at the first character or at the first wordbreak, when we have not seen any compositionstart/end or IME-related events. David, I know that we can't reliably detect when an IME has closed, but maybe we can detect that no IME interaction whatsoever is happening? Or are there IMEs that fly under the radar so much that we can't detect that they're being used?
* Throw the towel in the right and change our link continuation logic to either 'always' or 'never'. I don't really like solving a problem by redefining it to be correct behavior, and I also don't think it would be a good user experience, but IIRc this was David's first suggestion when confronted with this general problem.

Oh also: the bug as filed (select link, start typing, replacement text is linked in CE but not in DM) also appears to be caused by the unicorn change, presumably because such replacements are now considered "simple" rather than "complex" so as to not get in the way of IMEs that replace text?

But I'm widening the scope of this bug to "Link continuation is broken".

*** Bug 72261 has been marked as a duplicate of this bug. ***

(In reply to Roan Kattouw from comment #3)
> Oh also: the bug as filed (select link, start typing, replacement text is
> linked in CE but not in DM) also appears to be caused by the unicorn change,
> presumably because such replacements are now considered "simple" rather than
> "complex" so as to not get in the way of IMEs that replace text?

Actually, I think it's just due to the basic issue with link continuation when typing. The change of the linked word (<a href="x">before</a>) to the first character typed (<a href="x">a</a>) works correctly. But then the continuation does not occur, so we end up with just the first character linked (<a href="x">a</a>fter).

I tell a lie, in Chromium the behaviour is as Roan describes for the first letter, i.e. linked in the DOM but not in DM (but then subsequent letters are not linked the DOM either). In Firefox, there is no link in either the DOM or in DM.

https://gerrit.wikimedia.org/r/168327/ is a POC fix to the problem of right-cursoring to a continuation position, and it also illustrates the general issues in the other cases:

1. Unicorning is triggered by DM selection changes only if ve.dm.Surface 'insertionAnnotationsChange' is fired: that's how ve.ce.Surface renderSelectedContentBranchNode gets called. But moving to a continuation position doesn't necessarily mean the insertion annotations will have changed.

2. DOM-originated selection changes are applied to the DM with a render lock in place. This has to be so, because there are more DOM cursor positions than DM ones.

3. Therefore, perhaps we should make continuation unicorning happen even if there is a render lock in place, probably with its own emit event.

4. The DM needs to know for which annotations will browser continuation suffice. The merged code in ve.ce.ContentBranchNode getRenderedContents special-cases links only, whereas the patchset above adds a property to the annotation classes.

Continuation unicorns can give us short-term consistency, but not the desired "word-break" behaviour. If we want to do go with the short-term fix, we should define exactly what behaviour to implement. In the longer term, a UI change might provide a better experience -- e.g. show the user more explicitly that they're inside link continuation and let them cursor/click out of it.

(In reply to D Chan from comment #7)
> 1. Unicorning is triggered by DM selection changes only if ve.dm.Surface
> 'insertionAnnotationsChange' is fired: that's how ve.ce.Surface
> renderSelectedContentBranchNode gets called. But moving to a continuation
> position doesn't necessarily mean the insertion annotations will have
> changed.
> 
> 2. DOM-originated selection changes are applied to the DM with a render lock
> in place. This has to be so, because there are more DOM cursor positions
> than DM ones.
> 
> 3. Therefore, perhaps we should make continuation unicorning happen even if
> there is a render lock in place, probably with its own emit event.
> 
Both #1 and #2 are good reasons for #3. Maybe this could be done in setSelection()?

> 4. The DM needs to know for which annotations will browser continuation
> suffice. The merged code in ve.ce.ContentBranchNode getRenderedContents
> special-cases links only, whereas the patchset above adds a property to the
> annotation classes.
> 
Yeah there is a CE property for this (forceContinuation), but I guess my proposed solution to #3 may require the DM know. I would still prefer, though, to find a way to keep this knowledge in CE, possibly by having DM emitting too many events that are then filtered by CE.

> Continuation unicorns can give us short-term consistency, but not the
> desired "word-break" behaviour. If we want to do go with the short-term fix,
> we should define exactly what behaviour to implement. In the longer term, a
> UI change might provide a better experience -- e.g. show the user more
> explicitly that they're inside link continuation and let them cursor/click
> out of it.
Yeah we have talked about this in the past. I will try to find this and show it to you, it's scarily analogous to unicorns (before unicorns were invented).

Change 168327 had a related patch set uploaded by Catrope:
Make setSelection emit activeAnnotationChange if continuation unicorns may be needed

https://gerrit.wikimedia.org/r/168327

*** Bug 72638 has been marked as a duplicate of this bug. ***