Use array_merge() for OutputPage::$mLanguageLinks, not +
authorAryeh Gregor <ayg@aryeh.name>
Wed, 25 Jul 2018 18:36:56 +0000 (21:36 +0300)
committerAryeh Gregor <ayg@aryeh.name>
Sun, 29 Jul 2018 11:21:10 +0000 (14:21 +0300)
Based on documentation together with inspection of some callers, the
intent seems to be that this is an indexed array, not associative.  +
will therefore do totally the wrong thing, ignoring any new values that
have the same key as an existing item (e.g., '0' or '1').  Even if it
was an associative array, + keeps the values on the left-hand side,
which means you normally want to do $foo = $bar + $foo instead of $foo
+= $bar if you want to overwrite old values with the new ones.

Before this change, calling addLanguageLinks() or
addParserOutputMetadata() would generally not add all of the links it
was supposed to if there were already links defined.  (It could still
work if the arrays' keys didn't conflict for some reason, e.g.,
something passed an associative array or an indexed array with a hole.)
I don't know if anything actually hits this bug, because it's likely
that callers usually add all their links at once.  I find no uses of
addLanguageLinks() at all.

I found this bug while working on adding more tests for OutputPage, and
the tests for this change will be submitted later in
Icdc0288c04b8c4ba841f9fbb3e05a0cdc8a20fa5.

Change-Id: I53f6e7ea94417b0034371e56e733e8c86af21658

includes/OutputPage.php

index 4e7c0bb..25571a6 100644 (file)
@@ -1262,7 +1262,7 @@ class OutputPage extends ContextSource {
         *                               (e.g. 'fr:Test page')
         */
        public function addLanguageLinks( array $newLinkArray ) {
-               $this->mLanguageLinks += $newLinkArray;
+               $this->mLanguageLinks = array_merge( $this->mLanguageLinks, $newLinkArray );
        }
 
        /**
@@ -1800,7 +1800,8 @@ class OutputPage extends ContextSource {
         * @param ParserOutput $parserOutput
         */
        public function addParserOutputMetadata( $parserOutput ) {
-               $this->mLanguageLinks += $parserOutput->getLanguageLinks();
+               $this->mLanguageLinks =
+                       array_merge( $this->mLanguageLinks, $parserOutput->getLanguageLinks() );
                $this->addCategoryLinks( $parserOutput->getCategories() );
                $this->setIndicators( $parserOutput->getIndicators() );
                $this->mNewSectionLink = $parserOutput->getNewSection();