Page MenuHomePhabricator

After changing a link target, the link is selected such that typing will delete the link
Closed, ResolvedPublic

Description

  1. Change a link's target via the inspector.
  2. Notice the link is selected.
  3. Press "a".
  4. Notice "a" is typed, and is no longer linked.

The link-selection should be within the link cartouche, such that typing replaces the link label rather than the entire link.

Event Timeline

JTannerWMF added a subscriber: JTannerWMF.

Is this something we should consider for next year's work or is it relatively easy to work on perhaps the end of Q3 or in Q4? @DLynch

It should be pretty simple to do -- I've had it on my list of tasks to knock out when I have 15 minutes free.

It gets lightly more complicated if I want to make it work generically for all annotations-with-inspectors, which I probably should. Still, quick ticket. Feel free to assign it to me, and it'll all be over by Christmas.

Change 482673 had a related patch set uploaded (by DLynch; owner: DLynch):
[VisualEditor/VisualEditor@master] ui.AnnotationInspector: tweak selection after adding annotation

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

I think the current approach is about right for links, which have "nails" anchoring the annotation boundaries – I'll review the details of the patch later on.

I'm not sure it will work reliably for annotations that don't have nails, because the browser can rewrite the selection in a way that crosses the annotation boundary (how aggressively depends on the browser). I'll do some testing.

@dchan: I was thinking that the alternative route was to write greater awareness of nails into selections, such that when setting a selection you could declare whether it should bias inside or outside. That'd avoid this whole post-selection-change fixup routine... but would also complicate the core selection-handling code for the comparatively-unusual case. (That said, I don't have a comprehensive set of cases in my head for model-equivalent-but-practically-different selections. It's possible it'd be applicable to enough cases that it'd make sense to move further into the core...)

@DLynch : Do you mean when setting up a view selection or a model selection?

To me the former seems more feasible than the latter. I did once (c. 2014?) look at making the model selection more granular, to correspond more closely to the DOM selection, by recording on which side of an annotation boundary the anchor/focus lay. Ultimately I abandoned this, because the way the browser can rewrite the annotation nesting under your feet makes this not very meaningful. For example, you might encode in the model that the cursor is here: <b>[cursor]<i>foo</i></b> ... but then you blink and discover the browser has effectively rewritten the HTML as <i><b>foo</b></i>. With nailed annotations (which didn't exist back in 2014), the nesting is well-defined, so you could actually make this work – but I think it would mean having a totally different type of model representation for nailed annotations only, which seems pretty drastic.

@dchan: In this case it'd need to be model selections -- the annotation inspectors deal entirely with changes to the model. There's a reason I went for the somewhat-hackier route of just tweaking the DOM selection after the normal annotation path had been taken. :D

Change 482673 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] ui.AnnotationInspector: tweak selection after adding annotation

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

Let's go forward with your patch, because it's simple, neat and works fine for links, which in practice are by far the most significant case.

Change 485866 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (5883578a8)

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

Change 485866 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (5883578a8)

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