Page MenuHomePhabricator

Selection after auto-converting a basic (autonumbered) link to a regular external link selects outside the pin, meaning typing deletes the annotation, if it's not at the start of a <p>.
Closed, ResolvedPublic8 Story Points

Description

I put 2 autonumbered links in my sandbox. I then VEdited the page to add labels using the "new" button in the link dialog. It worked fine with the first one, but when I tried to add it to the second, as soon as I type the first letter the dialog closes, and the link vanishes.
I thought it had something to do with the links pointing to Phabricator, for which we have an interwiki shortcut, so I tried with two different URLs, but the result didn't change. (You can test here if you want.)

Details

Related Gerrit Patches:
mediawiki/extensions/VisualEditor : masterWhen adding a label to a numbered link, force the selection
VisualEditor/VisualEditor : masterAdd ve.ce.Surface selectActiveLinkContents

Event Timeline

Elitre created this task.Oct 5 2015, 1:14 PM
Elitre raised the priority of this task from to Needs Triage.
Elitre updated the task description. (Show Details)
Elitre added a project: VisualEditor.
Elitre added a subscriber: Elitre.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 5 2015, 1:14 PM
Elitre updated the task description. (Show Details)Oct 5 2015, 1:17 PM
Elitre set Security to None.
Jdforrester-WMF renamed this task from When adding a label to two consecutive autonumbered links, the second link disappears to Selection after auto-converting a basic (autonumbered) link to a regular external link selects outside the pin, meaning typing deletes the annotation, if it's not at the start of a <p>..Oct 6 2015, 7:10 PM
Jdforrester-WMF triaged this task as Medium priority.
Jdforrester-WMF edited a custom field.
Jdforrester-WMF moved this task from To Triage to Needs Discussion/Analysis on the VisualEditor board.
Jdforrester-WMF added a subscriber: dchan.

Change 254613 had a related patch set uploaded (by DLynch):
Add ve.ce.Surface selectActiveLinkContents

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

Change 254615 had a related patch set uploaded (by DLynch):
When adding a label to a numbered link, force the selection

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

Esanders added a subscriber: Esanders.EditedNov 21 2015, 1:03 AM

This is related to T114376. I was thinking of a more general fix that allowed us to specify an inner or outer bias when setting a model selection (outer bias meaning go to the 'outside' of a tag that lives within your dm selection offset).

DLynch added a subscriber: DLynch.Nov 21 2015, 1:13 AM

I was initially thinking that might be a bit ambiguous, just because the annotations can pile and we could have <b><i><a>foo. But if we don't mind ignoring the "I want to put the cursor between the <b> and the <i> case, it'd be relatively consistent to just have, essentially a "at the start of offset X" and "at the end of offset X" hint.

Yeah I think inner/outer is crude but enough for the bugs we have now.

When you set a cursor position in Chromium, it will silently get normalized to the "start-most" equivalent position with respect to annotation tags and text nodes.

For example if you tell it to select '<p><b>|<text#>foo</#text>|</b></p>' (i.e. just the content of the bold element) then it will actually select '<p>|<b><text#>foo|</#text></b></p>' (i.e. starting just before the bold element, and ending at the last offset of the text node).

So a function purporting to bias a general model selection would at times fail silently. On the other hand, specific code to select the link content will work on all browsers, because of link nails. Therefore I'd prefer the latter.

Change 254613 merged by jenkins-bot:
Add ve.ce.Surface selectActiveLinkContents

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

What about "nail bias" for links and anything that uses nails in the future. That would fix T114376 too.

Change 254615 merged by jenkins-bot:
When adding a label to a numbered link, force the selection

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