diff --git a/includes/changetags/ChangeTags.php b/includes/changetags/ChangeTags.php index 8ce9a148f4e..0864bb14614 100644 --- a/includes/changetags/ChangeTags.php +++ b/includes/changetags/ChangeTags.php @@ -168,22 +168,21 @@ class ChangeTags { * Creates HTML for the given tags * * @param string $tags Comma-separated list of tags - * @param string $page A label for the type of action which is being displayed, - * for example: 'history', 'contributions' or 'newpages' - * @param IContextSource|null $context - * @note Even though it takes null as a valid argument, an IContextSource is preferred + * @param string $page Unused + * @param MessageLocalizer|null $localizer + * @note Even though it takes null as a valid argument, a MessageLocalizer is preferred * in a new code, as the null value is subject to change in the future * @return array Array with two items: (html, classes) * - html: String: HTML for displaying the tags (empty string when param $tags is empty) * - classes: Array of strings: CSS classes used in the generated html, one class for each tag * @return-taint onlysafefor_htmlnoent */ - public static function formatSummaryRow( $tags, $page, IContextSource $context = null ) { - if ( !$tags ) { + public static function formatSummaryRow( $tags, $page, MessageLocalizer $localizer = null ) { + if ( $tags === '' ) { return [ '', [] ]; } - if ( !$context ) { - $context = RequestContext::getMain(); + if ( !$localizer ) { + $localizer = RequestContext::getMain(); } $classes = []; @@ -196,11 +195,11 @@ class ChangeTags { $displayTags = []; foreach ( $tags as $tag ) { - if ( !$tag ) { + if ( $tag === '' ) { continue; } $classes[] = Sanitizer::escapeClass( "mw-tag-$tag" ); - $description = self::tagDescription( $tag, $context ); + $description = self::tagDescription( $tag, $localizer ); if ( $description === false ) { continue; } @@ -216,7 +215,7 @@ class ChangeTags { return [ '', $classes ]; } - $markers = $context->msg( 'tag-list-wrapper' ) + $markers = $localizer->msg( 'tag-list-wrapper' ) ->numParams( count( $displayTags ) ) ->rawParams( implode( ' ', $displayTags ) ) ->parse(); @@ -358,8 +357,18 @@ class ChangeTags { &$rev_id = null, &$log_id = null, $params = null, RecentChange $rc = null, UserIdentity $user = null ) { - $tagsToAdd = array_filter( (array)$tagsToAdd ); // Make sure we're submitting all tags... - $tagsToRemove = array_filter( (array)$tagsToRemove ); + $tagsToAdd = array_filter( + (array)$tagsToAdd, // Make sure we're submitting all tags... + static function ( $value ) { + return ( $value ?? '' ) !== ''; + } + ); + $tagsToRemove = array_filter( + (array)$tagsToRemove, + static function ( $value ) { + return ( $value ?? '' ) !== ''; + } + ); if ( !$rc_id && !$rev_id && !$log_id ) { throw new MWException( 'At least one of: RCID, revision ID, and log ID MUST be ' . @@ -876,7 +885,7 @@ class ChangeTags { * @param string|array &$conds Conditions used in query, see Database::select * @param array &$join_conds Join conditions, see Database::select * @param string|array &$options Options, see Database::select - * @param string|array $filter_tag Tag(s) to select on (OR) + * @param false|string|array $filter_tag Tag(s) to select on (OR) * @param bool $exclude If true, exclude tag(s) from $filter_tag (NOR) * * @throws MWException When unable to determine appropriate JOIN condition for tagging @@ -907,7 +916,11 @@ class ChangeTags { throw new MWException( 'Unable to determine appropriate JOIN condition for tagging.' ); } - if ( $useTagFilter && $filter_tag ) { + if ( !$useTagFilter ) { + return; + } + + if ( $filter_tag !== '' && $filter_tag !== false && $filter_tag !== [] ) { // Somebody wants to filter on a tag. // Add an INNER JOIN on change_tag $tagTable = self::getDisplayTableName(); @@ -1654,7 +1667,7 @@ class ChangeTags { $fname ); - return array_filter( array_unique( $tags ) ); + return array_unique( $tags ); }, [ 'checkKeys' => [ $cache->makeKey( 'valid-tags-db' ) ], @@ -1689,7 +1702,7 @@ class ChangeTags { $setOpts += Database::getCacheSetOptions( wfGetDB( DB_REPLICA ) ); $hookRunner->onListDefinedTags( $tags ); - return array_filter( array_unique( $tags ) ); + return array_unique( $tags ); }, [ 'checkKeys' => [ $cache->makeKey( 'valid-tags-hook' ) ], diff --git a/includes/logging/LogPager.php b/includes/logging/LogPager.php index 3b10ade5af0..b3362aa6f98 100644 --- a/includes/logging/LogPager.php +++ b/includes/logging/LogPager.php @@ -116,7 +116,7 @@ class LogPager extends ReverseChronologicalPager { $this->limitTitle( $page, $pattern ); $this->limitAction( $action ); $this->getDateCond( $year, $month, $day ); - $this->mTagFilter = $tagFilter; + $this->mTagFilter = (string)$tagFilter; } public function getDefaultQuery() { @@ -376,7 +376,7 @@ class LogPager extends ReverseChronologicalPager { // `logging` and filesorting is somehow better than querying $limit+1 rows from `logging`. // Tell it not to reorder the query. But not when tag filtering or log_search was used, as it // seems as likely to be harmed as helped in that case. - if ( !$this->mTagFilter && !array_key_exists( 'ls_field', $this->mConds ) ) { + if ( $this->mTagFilter === '' && !array_key_exists( 'ls_field', $this->mConds ) ) { $options[] = 'STRAIGHT_JOIN'; } diff --git a/includes/specials/SpecialRecentChanges.php b/includes/specials/SpecialRecentChanges.php index 7288d0d3e85..87687ee189f 100644 --- a/includes/specials/SpecialRecentChanges.php +++ b/includes/specials/SpecialRecentChanges.php @@ -366,7 +366,7 @@ class SpecialRecentChanges extends ChangesListSpecialPage { $fields[] = 'page_latest'; $join_conds['page'] = [ 'LEFT JOIN', 'rc_cur_id=page_id' ]; - $tagFilter = $opts['tagfilter'] ? explode( '|', $opts['tagfilter'] ) : []; + $tagFilter = $opts['tagfilter'] !== '' ? explode( '|', $opts['tagfilter'] ) : []; ChangeTags::modifyDisplayQuery( $tables, $fields, diff --git a/includes/specials/SpecialRecentChangesLinked.php b/includes/specials/SpecialRecentChangesLinked.php index 94ce9e0a4eb..1cef4f7b6d5 100644 --- a/includes/specials/SpecialRecentChangesLinked.php +++ b/includes/specials/SpecialRecentChangesLinked.php @@ -127,7 +127,7 @@ class SpecialRecentChangesLinked extends SpecialRecentChanges { $join_conds['page'] = [ 'LEFT JOIN', 'rc_cur_id=page_id' ]; $select[] = 'page_latest'; - $tagFilter = $opts['tagfilter'] ? explode( '|', $opts['tagfilter'] ) : []; + $tagFilter = $opts['tagfilter'] !== '' ? explode( '|', $opts['tagfilter'] ) : []; ChangeTags::modifyDisplayQuery( $tables, $select, diff --git a/includes/specials/SpecialWatchlist.php b/includes/specials/SpecialWatchlist.php index a07b93dd068..9a3ee7b3cad 100644 --- a/includes/specials/SpecialWatchlist.php +++ b/includes/specials/SpecialWatchlist.php @@ -435,7 +435,7 @@ class SpecialWatchlist extends ChangesListSpecialPage { ], LIST_OR ); } - $tagFilter = $opts['tagfilter'] ? explode( '|', $opts['tagfilter'] ) : []; + $tagFilter = $opts['tagfilter'] !== '' ? explode( '|', $opts['tagfilter'] ) : []; ChangeTags::modifyDisplayQuery( $tables, $fields, diff --git a/tests/phpunit/includes/changetags/ChangeTagsTest.php b/tests/phpunit/includes/changetags/ChangeTagsTest.php index 0834426afd9..eeed15b46af 100644 --- a/tests/phpunit/includes/changetags/ChangeTagsTest.php +++ b/tests/phpunit/includes/changetags/ChangeTagsTest.php @@ -661,4 +661,62 @@ class ChangeTagsTest extends MediaWikiIntegrationTestCase { [ 'ORDER BY' => 'ctd_name' ] ); } + + public function provideFormatSummaryRow() { + yield 'nothing' => [ '', [ '', [] ] ]; + yield 'valid tag' => [ + 'tag1', + [ + '(tag-list-wrapper: 1, ' + . '(tag-tag1)' + . ')', + [ 'mw-tag-tag1' ] + ] + ]; + yield '0 tag' => [ + '0', + [ + '(tag-list-wrapper: 1, ' + . '(tag-0)' + . ')', + [ 'mw-tag-0' ] + ] + ]; + yield 'hidden tag' => [ + 'hidden-tag', + [ + '', + [ 'mw-tag-hidden-tag' ] + ] + ]; + yield 'mutliple tags' => [ + 'tag1,0,,hidden-tag', + [ + '(tag-list-wrapper: 2, ' + . '(tag-tag1)' + . ' (tag-0)' + . ')', + [ 'mw-tag-tag1', 'mw-tag-0', 'mw-tag-hidden-tag' ] + ] + ]; + } + + /** + * @dataProvider provideFormatSummaryRow + */ + public function testFormatSummaryRow( $tags, $expected ) { + $qqx = new MockMessageLocalizer(); + $localizer = $this->createMock( MessageLocalizer::class ); + $localizer->method( 'msg' ) + ->willReturnCallback( static function ( $key, ...$params ) use ( $qqx ) { + if ( $key === 'tag-hidden-tag' ) { + return new RawMessage( '-' ); + } + return $qqx->msg( $key, ...$params ); + } ); + + $out = ChangeTags::formatSummaryRow( $tags, 'dummy', $localizer ); + $this->assertSame( $expected, $out ); + } + } diff --git a/tests/phpunit/includes/pager/HistoryPagerTest.php b/tests/phpunit/includes/pager/HistoryPagerTest.php index ea298c3faf5..c53fcc26ad0 100644 --- a/tests/phpunit/includes/pager/HistoryPagerTest.php +++ b/tests/phpunit/includes/pager/HistoryPagerTest.php @@ -103,7 +103,7 @@ class HistoryPagerTest extends MediaWikiLangTestCase { [ [ 'rev_page' => 'title', - 'ts_tags' => [], + 'ts_tags' => '', 'rev_deleted' => false, 'rev_minor_edit' => false, 'rev_parent_id' => 1, @@ -128,7 +128,7 @@ class HistoryPagerTest extends MediaWikiLangTestCase { 'rev_page' => 'title', 'rev_deleted' => false, 'rev_minor_edit' => false, - 'ts_tags' => [], + 'ts_tags' => '', 'rev_parent_id' => 1, 'user_name' => 'Jdlrobson', 'rev_comment_data' => '{}', @@ -173,7 +173,7 @@ class HistoryPagerTest extends MediaWikiLangTestCase { 'rev_page' => 'title', 'rev_deleted' => false, 'rev_minor_edit' => false, - 'ts_tags' => [], + 'ts_tags' => '', 'rev_parent_id' => 1, 'user_name' => 'Jdlrobson', 'rev_comment_data' => '{}',