Screenshot
Steps to reproduce:
1.Open a page with VE
2.Select a text and apply link on it
3.Now while link is selected, type something else to replace the link text.
Observed Result:
The previous link is getting applied only to the first letter of the newly added text
See the screenshot attached
Environment:Betalabs
Version: unspecified
Severity: major
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=72261
Attached:
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | dchan | T74108 [Regression wmf4] Link continuation is broken | |||
| Resolved | dchan | T91285 Change how link annotations work to actively enter/leave the annotation |
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:
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".
(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:
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)
- 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.
- 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.
- 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()?
- 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).
gerritadmin wrote:
Change 168327 had a related patch set uploaded by Catrope:
Make setSelection emit activeAnnotationChange if continuation unicorns may be needed
@dchan/@Jdforrester-WMF, is the issue described in T102919 related to recent work in link continuation area?
Change 217257 had a related patch set uploaded (by Divec):
Explicitly enter and exit link annotations
Commit a32524705038bfc832bcd0e04e2e4e6322b080f5 (heed cursor position relative to annotations) has bearing on this, in that if we ripped out the special casing for links, the native behaviour of Firefox would just work. However, Chromium aggressively discontinues links, so getting reasonable and consistent behaviour still needs T91285 .