install_find_translation_files() breaks if $langcode is passed

You may need to upload a patch file without the "-D8" suffix in order to get the testbot to test it, but the patch looks good to me.

Minor issues with the PHPDoc, though: The description of $langcode should probably use the "(optional)" phrasing, and there should be a blank line before @return (see http://drupal.org/node/1354).

+ * @param $langcode
+ *   (optional) The language code corresponding to the language for which we
+ *   want to find translation files; or NULL to find all available translation
+ *   files.

Sorry to nitpick, but the second part (after the semicolon) doesn't look quite right to me. It would probably match existing documentation better if it were a completely new sentence, something more like "If omitted, all available translation files will be returned"?

@Garrett: First of all, nice find. Thanks for the patch! Now my comments. The code looks good, comments have some issues still.

1. "An array of information about found translation files." is not really descriptive. Array of information can really be anything. I'd add "as returned by file_scan_directory() or somesuch". I don't think this comment adds anything useful at the moment.

2. "Ommitted" should be spelled "Omitted" AFAIK.

New patch as per my suggestions from yesterday.

Looks like we could add some dummy files to a test directoy, change locale_translate_file_directory to to point to that, and write a test for this?

Moving to needs work for that.

Here is a comprehensive test that proves it fails without the patch and a compound patch that should pass (they failed and passed proper on my machine). Please review!

Removed an incorrectly copy-pasted code comment. No other changes, so not uploading the fail version again :)

Ok, testbot proof ready. Human reviews welcome :)

@catch: what do you think of the added tests? Looks good now to go?

Looks good.

Sorry to bump this back for a few small things, but...

+files[] = tests/installer.test
 files[] = tests/image.test

Wrong alphabetical order :)

+class InstallerLanguageTestCase extends DrupalWebTestCase {
+
+  public static function getInfo() {
+    return array(
+      'name' => 'Installer tests',
+      'description' => 'Unit tests for the installer.',

The class needs some PHPDoc above it.

Also, the name of the class ("InstallerLanguageTestCase") is specific to the language tests, but the getInfo() stuff refers to much more general-sounding tests... so let's pick one or the other. I'd probably go with keeping it all language-specific for the time being, since the setUp() method is doing something that is specific to the language tests.

+  /**
+   * Check whether translation files are found for the installer.
+   */

At least needs to be "Checks" rather than "Check"... but I'd probably go with something more like "Tests that the installer can find translation files"?

Ok, all concerns fixed. Good now?

Also elevating this. The bug basically means the installer will never be translated even if you select a language, because then it cannot actually find the translation file in subsequent requests. The patch fixes that.

Looks good. Technically, "Test installer language detection" should actually be "Tests installer language detection", but I'm not going to prevent it from sitting in the RTBC state for that.

However, if you rerolled with that change, the coding standards people will be happier :)

Rerolled for that.

EDIT: No other changes, so leaving at RTBC.

Untagging 'cause it has tests.

Looks good with the new tests. Committed/pushed to 8.x.

It looks like installer.test wasn't added.

Not sure if we need a new patch for that or not?... (it should be possible to just reapply the patch above and ignore all the parts that are already there, then add the new file and commit).

Arggh, well spotted!

Committed/pushed that file to 8.x.

+++ b/core/includes/install.core.incundefined
@@ -1159,10 +1159,21 @@ function install_find_translations() {
diff --git a/core/modules/simpletest/files/translations/install.de.po b/core/modules/simpletest/files/translations/install.de.po

diff --git a/core/modules/simpletest/files/translations/install.de.po b/core/modules/simpletest/files/translations/install.de.po
new file mode 100644
index 0000000..e69de29

index 0000000..e69de29
diff --git a/core/modules/simpletest/files/translations/install.hu.po b/core/modules/simpletest/files/translations/install.hu.po

diff --git a/core/modules/simpletest/files/translations/install.hu.po b/core/modules/simpletest/files/translations/install.hu.po
new file mode 100644
index 0000000..e69de29

These files are also missing.

Bumping to critical because branch tests are failing atm.

RTBC since we have the files in the patch? Don't know what else can we do here minus core maintainer action to commit them.

catch fixed it just now. :)

Oops. Apparently I didn't spot it well enough :)

Adding UI language translation tag.