Fix internal presentation of the dir="…" attribute

This fixes a whole bunch of inconsistencies:

* The dir attribute is now trimmed, as most others already are. This is
an actual user-facing change.

* The internal representation is now false in case the value was invalid,
not an empty string any more.

* Null means the attribute was not present. This is now always used,
even in the return values that are meant to represent an error state. No
existing behavior changes.

* The internal representation does not contain an HTML snippet any more,
but the raw value "ltr" or "rtl", or null. Note this might influence the
API, because the API actually exposes the internal representation.
However, we are pretty sure the API is not used anywhere. Even if,
exposing HTML code was most certainly an unwanted and unexpected effect
of the patch that introduced the dir attribute. This does make this a
bugfix, I would argue.

Change-Id: Ic385d9ab36fa0545c374d3d63063028ae4e449d4
This commit is contained in:
Thiemo Kreuz 2019-11-21 12:52:47 +01:00
parent ab3063fee5
commit 65c8967c32
2 changed files with 18 additions and 21 deletions

View file

@ -244,8 +244,8 @@ class Cite {
) {
# The key here is the "name" attribute.
list( $key, $group, $follow, $dir, $extends ) = $this->refArg( $argv );
// empty string indicate invalid dir
if ( $dir === '' && $text !== '' ) {
if ( $dir === false && $text !== '' ) {
$text .= $this->errorReporter->wikitext( 'cite_error_ref_invalid_dir', $argv['dir'] );
}
# Split these into groups.
@ -417,13 +417,10 @@ class Cite {
$extends = null;
if ( isset( $argv['dir'] ) ) {
// compare the dir attribute value against an explicit whitelist.
$dir = '';
$isValidDir = in_array( strtolower( $argv['dir'] ), [ 'ltr', 'rtl' ] );
if ( $isValidDir ) {
$dir = Html::expandAttributes( [ 'class' => 'mw-cite-dir-' . strtolower( $argv['dir'] ) ] );
$dir = strtolower( trim( $argv['dir'] ) );
if ( !in_array( $dir, [ 'ltr', 'rtl' ] ) ) {
$dir = false;
}
unset( $argv['dir'] );
}
@ -435,7 +432,7 @@ class Cite {
if ( isset( $argv['follow'] ) &&
( isset( $argv['name'] ) || isset( $argv[self::BOOK_REF_ATTRIBUTE] ) )
) {
return [ false, false, false, false, false ];
return [ false, false, false, null, false ];
}
if ( isset( $argv['name'] ) ) {
@ -460,7 +457,7 @@ class Cite {
if ( $argv !== [] ) {
// Unexpected invalid attribute.
return [ false, false, false, false, false ];
return [ false, false, false, null, false ];
}
return [ $key, $group, $follow, $dir, $extends ];
@ -837,7 +834,7 @@ class Cite {
$this->normalizeKey( self::getReferencesKey( $id ) ),
$this->normalizeKey( $backlinkId ),
$text,
$val['dir']
$val['dir'] ? Html::expandAttributes( [ 'class' => 'mw-cite-dir-' . $val['dir'] ] ) : ''
)->inContentLanguage()->plain();
}
@ -861,7 +858,7 @@ class Cite {
),
$this->listToText( $backlinks ),
$text,
$val['dir']
$val['dir'] ? Html::expandAttributes( [ 'class' => 'mw-cite-dir-' . $val['dir'] ] ) : ''
)->inContentLanguage()->plain();
}

View file

@ -35,27 +35,27 @@ class CiteTest extends \MediaWikiUnitTestCase {
[ [], [ null, null, false, null, null ] ],
// One attribute only
[ [ 'dir' => 'invalid' ], [ null, null, false, '', null ] ],
[ [ 'dir' => 'rtl' ], [ null, null, false, ' class="mw-cite-dir-rtl"', null ] ],
[ [ 'dir' => 'invalid' ], [ null, null, false, false, null ] ],
[ [ 'dir' => ' rtl ' ], [ null, null, false, 'rtl', null ] ],
[ [ 'follow' => ' f ' ], [ null, null, 'f', null, null ] ],
// FIXME: Unlike all other attributes, group isn't trimmed. Why?
[ [ 'group' => ' g ' ], [ null, ' g ', null, null, null ] ],
[ [ 'invalid' => 'i' ], [ false, false, false, false, false ] ],
[ [ 'invalid' => null ], [ false, false, false, false, false ] ],
[ [ 'invalid' => 'i' ], [ false, false, false, null, false ] ],
[ [ 'invalid' => null ], [ false, false, false, null, false ] ],
[ [ 'name' => ' n ' ], [ 'n', null, null, null, null ] ],
[ [ 'name' => null ], [ false, false, false, false, false ] ],
[ [ 'name' => null ], [ false, false, false, null, false ] ],
[ [ 'extends' => ' e ' ], [ null, null, null, null, 'e' ] ],
// Pairs
[ [ 'follow' => 'f', 'name' => 'n' ], [ false, false, false, false, false ] ],
[ [ 'follow' => null, 'name' => null ], [ false, false, false, false, false ] ],
[ [ 'follow' => 'f', 'extends' => 'e' ], [ false, false, false, false, false ] ],
[ [ 'follow' => 'f', 'name' => 'n' ], [ false, false, false, null, false ] ],
[ [ 'follow' => null, 'name' => null ], [ false, false, false, null, false ] ],
[ [ 'follow' => 'f', 'extends' => 'e' ], [ false, false, false, null, false ] ],
[ [ 'group' => 'g', 'name' => 'n' ], [ 'n', 'g', null, null, null ] ],
// Combinations of 3 or more attributes
[
[ 'group' => 'g', 'name' => 'n', 'extends' => 'e', 'dir' => 'rtl' ],
[ 'n', 'g', null, ' class="mw-cite-dir-rtl"', 'e' ]
[ 'n', 'g', null, 'rtl', 'e' ]
],
];
}