Page MenuHomePhabricator

Make VisualEditor compatible with "extends" parameter
Open, MediumPublic

Description

Motivation:
Although the extends functionality cannot be used with VisualEditor yet, it should also not break VisualEditor if a page containing the new notation is opened.

Task:
When an article that contains the extends-notation is opened/edited with VisualEditor we need to make sure that:

  • VisualEditor can be used without any restrictions
  • "extends" references are still shown in the new format

Note: This task is not about implementing support for the "refines" parameter in VisualEditor yet, it is just for making it compatible.

Background
The wish to improve the referencing of multiple parts of the same source was part of the German-speaking community's technical wishlist in 2013, 2015 and of the international technical wishlist in 2015. For more info, see T100645.

Event Timeline

Izno added a project: VisualEditor.
Lea_WMDE renamed this task from Make VisualEditor compatible with "refines" parameter to Make VisualEditor compatible with "extends" parameter.May 21 2017, 7:31 PM
Lea_WMDE updated the task description. (Show Details)
Addshore removed a subscriber: Addshore.Apr 3 2018, 9:09 AM

I had a quick look at the Parsoid code. What I found worries me a little bit:

  • Parsoid does indeed not call the Cite extension, but re-implements a bunch of extensions: https://phabricator.wikimedia.org/diffusion/GPAR/browse/master/src/Ext/
  • On the <references> tag, Parsoid considers the group and responsive attributes. Search for $refsOpts here: https://phabricator.wikimedia.org/diffusion/GPAR/browse/master/src/Ext/Cite/References.php$70
  • On the <ref> tag, Parsoid considers name, group, and dir. That's it. Search for ->attrs in this file: https://phabricator.wikimedia.org/diffusion/GPAR/browse/master/src/Ext/Cite/References.php$143
  • Note: For both the <ref> and the <references> tag the list of arguments is passed as an associative array to the two toDOM methods, very similar to how Cite behaves. However, for some reason the <ref> tag is converted to a DOM element before accessing any of the arguments, while <references> accesses the associative array directly. This might even be bogus. For example, it looks like the group attribute is sanitized on <ref>, but not on <references>, possibly resulting in mismatching group names, depending on what the Parsoid sanitizer actually does.
  • follow does not exist. Parsoid ignores it and does not merge consecutive <ref> that use follow, but renders them as separate items. This does not only mean that the list of references at the end of the article lists more items. This also means a footnote marker will appear in the text where Cite doesn't show one. And all the numbers are different from this point on.
  • Parsoid does not do much error checking. For example, invalid attributes are not reported with a red error in the text. This is not consistent with the behavior of Cite. For us it's a good thing because it means Parsoid will ignore extends and again render such <ref> as separate items.
Izno added subscribers: ssastry, Izno.Wed, Jan 8, 6:49 PM

I honestly see the implementation of citation content (and other extensions) in Parsoid as technical debt to be cleaned up/removed eventually by the 1-parser-march. @ssastry should probably comment on the general plan for those extension reimplementations, if that has been considered previously (and if not, maybe it should be discussed somewhere more general).

I honestly see the implementation of citation content (and other extensions) in Parsoid as technical debt to be cleaned up/removed eventually by the 1-parser-march. @ssastry should probably comment on the general plan for those extension reimplementations, if that has been considered previously (and if not, maybe it should be discussed somewhere more general).

This is covered by T51538: Cite: Improve compatibility between Parsoid's port and the PHP extension. As part of our push to integrate parsers, we'll resolve that bug, but all said and done, Parsoid's Cite implementation is going to be the authoritative one eventually since it is the computation model and extension API that we are going to expose during the parser integration process. But, at that time, Parsoid's implementation will get moved to the Cite extension repo and will no longer live in the Parsoid repo.

But, see T110909: Parsoid tripped up by extensions that process wikitext as well. This year, we are going to start working on the parser integration work, and last year, we thought that by now, we would have a first draft of the new proposed parser hooks / Parsoid extension API, but we are a bit behind at this point since the Parsoid PHP port took longer and we still have some post-port followup work to finish ( T239660 being the big one ). So, it might still be a quarter before we have the Parsoid extension API proposal for review. We also have to get a Parser API resolved as well and that will go hand in hand with the extension API.

Following on from our discussion earlier, it should be possible to warn users editing sub-references, or disable them from being editing. Parsoid will pass through the "extends" attribute in the data-mw already, so in the VE model for the referenceNode, we can look at model.attrs.extends.

An example "fix" would be to return an "alienInline" node instead of an "mwReferenceNode" by adding the following to ve.dm.MWReferenceNode.static.toDataElement

	if ( mwData.attrs && mwData.attrs.extends ) {
		return { type: 'alienInline' };
	}

This would make sub-references completely uneditable. I don't think this would be a great experience, it would probably be better to modify the CE node and the ContextItem to show a warning, but it's a good place to start.

Another failure case that is going to be harder to handle is what to do if the user deletes a root reference without deleting its sub-references.

Sounds great, thanks a lot for the additional information! Another option might be to use the already available ….attrs.extends value to access the parent, and display it's contents while editing the sub-reference.

Another option might be to use the already available ….attrs.extends value to access the parent, and display it's contents while editing the sub-reference.

This would be my preferred implementation. But we haven't done UX work yet.