Page MenuHomePhabricator

Model and view out of sync when link deleted and replaced with text
Closed, ResolvedPublic8 Story Points

Description

It is really great how VE now communicates whether you are inside the link context or outside of it as you edit a link element.

It works quite well in general but I found a small issue worth documenting.
When adding an external link by pasting it into the document, the next action is to rename the label. If the label is removed character by character until the item is empty you can still add the new label. But as soon you continue editing somewhere else, the link gets lost.

I captured that in an animated gif for more clarity:

I tried alternative approaches like editing the label first or even selecting it with a double-click, and both worked well.

Event Timeline

Pginer-WMF raised the priority of this task from to Needs Triage.
Pginer-WMF updated the task description. (Show Details)
Pginer-WMF added a project: VisualEditor.
Pginer-WMF added a subscriber: Pginer-WMF.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 8 2015, 11:25 AM
Pginer-WMF updated the task description. (Show Details)Oct 8 2015, 11:29 AM
Pginer-WMF set Security to None.
Elitre added a subscriber: Elitre.Oct 8 2015, 11:44 AM

(I wonder if T114643 is related?)

Esanders renamed this task from Link context lost after renaming external link to Model and view out of sync when link deleted and replaced with text.Oct 26 2015, 3:54 PM
Esanders assigned this task to dchan.

Not limited to link annotations, get the same issue with bold (in Chrome)

dchan added a comment.Oct 26 2015, 3:59 PM

Hmm, this is happening because the model's context-based annotation inference fails if the model context does not have any text with the link annotation.

It would be fixed by the approach of https://gerrit.wikimedia.org/r/247800/ , but perhaps we should special-case this immediately by saving the annotation inside the link DOM element.

dchan added a comment.Oct 26 2015, 4:04 PM

BTW this looks like a special case of T116269 but isn't - the difference is in this case, the actual link element remains in the DOM, whereas in T116269, the browser-native preannotation system inserts a new, confusingly similar element.

Jdforrester-WMF removed dchan as the assignee of this task.Nov 19 2015, 7:11 PM

Still an issue after (the current version of) https://gerrit.wikimedia.org/r/247800/ unfortunately.

dchan added a subscriber: dchan.Dec 2 2015, 7:04 PM

It might be fixed if we set "ve.dm.LinkAnnotation.static.inferFromDom = true;" (on top of https://gerrit.wikimedia.org/r/247800/ ) - but I'd rather do that in a separate patch and check the consequences carefully before merging.

Change 257590 had a related patch set uploaded (by Esanders):
LinkAnnotation: Allow inference from DOM

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

Change 257613 had a related patch set uploaded (by Esanders):
WIP Recover annotations via .data('view').

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

Change 257613 merged by jenkins-bot:
TextState: Recover annotation models via .data('view').

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

Esanders closed this task as Resolved.Dec 8 2015, 5:41 PM
Esanders claimed this task.

Change 257590 abandoned by Esanders:
LinkAnnotation: Allow inference from DOM

Reason:
Replaced by https://gerrit.wikimedia.org/r/#/c/257613/

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

Cirdan added a subscriber: Cirdan.Feb 8 2016, 6:37 PM
MartinK added a subscriber: MartinK.Feb 8 2016, 7:05 PM