Page MenuHomePhabricator

Autolink doesn't take place when you finish a list item
Closed, ResolvedPublic1 Story Points

Description

If you type a URL or ISBN and press enter in a paragraph, the autolink code will be invoked when you press enter.

However, if you are in a list item (for example: at the bottom here ) and press enter, autolinking doesn't happen.

I think this is because autolinking stops as soon as it hits a "non plain text" element, like the </li><li>. IIRC there's a bit of a special case to make </p><p> work, but apparently something slightly different is needed for lists.

Event Timeline

cscott created this task.Sep 23 2015, 9:31 PM
cscott raised the priority of this task from to Needs Triage.
cscott updated the task description. (Show Details)
cscott added a project: VisualEditor.
cscott added a subscriber: cscott.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 23 2015, 9:31 PM

In VE, lists always contain paragraphs (or another content containing node, e.g. heading) as list items are non-content branch nodes.

Jdforrester-WMF triaged this task as Normal priority.Sep 24 2015, 4:46 PM
Jdforrester-WMF set Security to None.
Jdforrester-WMF edited a custom field.
Jdforrester-WMF moved this task from To Triage to TR0: Interrupt on the VisualEditor board.
Esanders added a comment.EditedSep 24 2015, 4:47 PM

The problem is the auto-link sequence is look for LinkRegex+Whitespace, which a newline isn't (it's a </paragraph> element). Fixing is not as simple as detecting LinkRegex+</paragraph> because that is also true before you press Enter, and while you still typing the link.

Conceptually you probably want to special case the user pressing enter somehow.

The special case that makes it work for paragraphs is https://github.com/wikimedia/VisualEditor/blob/master/src/ui/ve.ui.SequenceRegistry.js#L58

It's possible we can just extend that hack to handle lists, too. But I agree that manually triggering the sequence registry when enter is pressed (*before* the new list item/paragraph is inserted) would be a better fix (and the hack about could then be removed).

You'd have to be careful about the undo stack. Usually the sequence is "insert space", then "autolink", so that "undo" undoes the autolink first, then the "insert space". If we trigger the sequence registry before the insertion, then the first undo would undo the "enter", and you'd have to press undo a second time to undo the autolink. That inconsistency could be confusing.

Change 240910 had a related patch set uploaded (by Cscott):
Allow autolinking inside list items

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

Change 240911 had a related patch set uploaded (by Cscott):
Allow autolinking ISBN/PMID/RFC inside list items

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

Change 240910 merged by jenkins-bot:
Allow autolinking inside list items

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

Change 240911 merged by jenkins-bot:
Allow autolinking ISBN/PMID/RFC inside list items

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