Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-13868

Adding multiple language files for acp/mcp/ucp modules is incorrectly handled for extensions




      function add_mod_info() in includes/functions_module.php incorrectly handles modules lang files for extensions, namely (see line https://github.com/phpbb/phpbb/blob/3.1.x/phpBB/includes/functions_module.php#L1089):
      1) array_unique() doesn't allow using multiple info_Xcp_xxx.php language files per extension.
      2) the order in which language files are loaded is incorrect/suboptimal as $user_lang_files values are being overriden with $english_lang_files and $default_lang_files values in certain circumstances.

      IRC discussion:

      [01:09]	rxu: the thing is that if you have, say, 2 acp files, 2nd one will be filtered out
      [01:09]	rxu: for the 1 ext
      [01:10]	rxu: because values are the same (which are equal to the ext name)
      [01:11]	bantu: yeah
      [01:12]	bantu: array_merge() already does what is required
      [01:13]	bantu: even if multiple arrays contain the same map entry, the map entry will only be there once in the resulting array
      [01:13]	bantu: due to array_merge using keys
      [01:13]	rxu: indeed
      [01:18]	rxu: but getting rid of array_unique() is not enough as it leaads to overriding user lang values with the english or default (if any) ones
      [01:19]	bantu: but that is also the case currently?
      [01:20]	bantu: actually I don't know what finder returns
      [01:20]	bantu: absolute paths?
      [01:21]	rxu: not quite, it returns pairs like that
      [01:21]	rxu: 'ext/rxu/PostsMerging/language/en/info_acp_posts_merging.php' => string 'rxu/PostsMerging'
      [01:21]	rxu: 'string' part is a leftover of var_dump() output :P
      [01:21]	rxu: 'ext/rxu/PostsMerging/language/en/info_acp_posts_merging.php' => 'rxu/PostsMerging'
      [01:22]	bantu: hum
      [01:24]	bantu: i think these are different issues in any case
      [01:24]	bantu: the array_unique is clearly wrong
      [01:24]	bantu: consindering what array values are
      [01:25]	bantu: or hmm
      [01:25]	bantu: now I am confused
      [01:26]	bantu: maybe this was just intentional?
      [01:26]	bantu: as this is the legacy info file?
      [01:26]	bantu: but then we wouldn't be using finder
      [01:28]	rxu: yeah
      [01:29]	bantu: next I think you're saying that the order in which language files are loaded is incorrect/suboptimal
      [01:29]	bantu: I would probably agree with that.
      [01:30]	bantu: English, Default, User is probably better order
      [01:30]	bantu: array_merge should preserve the order
      [01:30]	rxu: exactly
      [01:31]	bantu: actually, array_merge never overwrites any keys as they are distinct.
      [01:33]	bantu: thus the array merge isn't actually necessary and we could just iterate over all three arrays seperately
      [01:37]	rxu: true, again
      [01:38]	rxu: although 1 foreach is more handy than 3 :P
      [01:39]	bantu: foreach with level 2 of course
      [01:39]	bantu: the array_merge is probably fine, though




            Marc Marc
            rxu rxu
            0 Vote for this issue
            2 Start watching this issue