Always wrap follows content

Avoids crashers from trying to serialize the named content twice if
there's a valid follow with some other error, as expected in the FIXME.

This introduces a backwards incompatibility for invalid follows, which
will result in the contents of the ref being dropped.

A test is added to assert that selser will save us for the most part.

Change-Id: I1f572f996a7c2b3b852752f5348ebb60d8e21c47
This commit is contained in:
Arlo Breault 2020-10-05 18:19:58 -04:00
parent 6bd0594f28
commit 1ac603ff1c
2 changed files with 12 additions and 15 deletions

View file

@ -151,12 +151,7 @@ class Ref extends ExtensionTagHandler {
$hasRefName = strlen( $dataMw->attrs->name ?? '' ) > 0;
$hasFollow = strlen( $dataMw->attrs->follow ?? '' ) > 0;
// FIXME: This isn't exactly right since a valid follow could
// potentially produce some other type of error, so this may
// need some more smarts
$validFollow = $hasFollow && !DOMUtils::hasTypeOf( $node, 'mw:Error' );
if ( $validFollow ) {
if ( $hasFollow ) {
$about = $node->getAttribute( 'about' );
$followNode = DOMCompat::querySelector(
$bodyElt, "span[typeof~='mw:Cite/Follow'][about='{$about}']"

View file

@ -173,6 +173,17 @@ class References extends ExtensionTagHandler {
$validFollow = false;
if ( $hasFollow ) {
// Always wrap follows content so that there's no ambiguity
// where to find it when roundtripping
$span = $doc->createElement( 'span' );
DOMUtils::addTypeOf( $span, 'mw:Cite/Follow' );
$span->setAttribute( 'about', $about );
$span->appendChild(
$doc->createTextNode( ' ' )
);
DOMUtils::migrateChildren( $c, $span );
$c->appendChild( $span );
if ( $hasRefName ) {
$errs[] = [ 'key' => 'cite_error_ref_too_many_keys' ];
} else {
@ -183,15 +194,6 @@ class References extends ExtensionTagHandler {
$validFollow = true;
$ref = $group->indexByName[$followName];
$span = $doc->createElement( 'span' );
DOMUtils::addTypeOf( $span, 'mw:Cite/Follow' );
$span->setAttribute( 'about', $about );
$span->appendChild(
$doc->createTextNode( ' ' )
);
DOMUtils::migrateChildren( $c, $span );
$c->appendChild( $span );
if ( $ref->contentId ) {
$refContent = $extApi->getContentDOM( $ref->contentId )->firstChild;
DOMUtils::migrateChildren( $c, $refContent );