When installing Drupal in a non-English language, we do not see the "congratulations" page.

@webflo looked into this. It is indeed seem like skipping the last screen. Probably due to how the batch in the last .po import process is structured.

Looks like the step to import the translation file is run twice, that's why it's jumping from the original step i think and return to standard batch display.
Removing one step seems to fix it.

Well, the role of the step at first is to import translations BEFORE configuration is saved, so we can save configuration with the translation (so you get translated node type names, shortcuts, etc). Then the translation import at the end of the installer is supposed to serve install profiles, so if they enabled new modules, we can import translations for those too.

Ok i see. I was thinking that the translation import file process was done in one go. I'm still looking into it. it is strange that the display change. Maybe there is a difference between the number of batch different steps and the global tasks count defined before or something. I also had some php language unset variables warnings in the logs.

Ok, spent some time but finally figure it out that the installation profile was kind of running in the middle, changing the theme and breaking the next steps. Now it's fine.it's when the update module is enabled, on submit that the config form is submit that it fails.

retrying

The previous patch is not a proper fix to this issue, just temporary. For a proper solution, install steps needs clarifications, especially on the import of .po files from the translation folder.
1. is the step "Set up translations" supposed to only import the main drupal core language po file for the language selected. If yes, that means that the batch import files functions from locale has to be tweaked to do so. To me it make sense that this step only import one main core gettext file.
2. Following this, the step "Finish translations" will import the other gettext files in the directory of modules enabled (that can be downloaded straight from localize if they are not physically in the folder) in a previous step.

To me, the batch import files of locale has to be modified. Right now, it imports all the gettext files in one go who have the language code selected in their filename.

after more investigation, it seems that it breaks on the config form submit where the update module is enable. Attached a patch showing this.

Attached an updated patch who actually separate the core main translation file from the other contribution translation files to get two proper steps of file import.
I think the config_form_submit function before "Finish translations" is changing the default conf of the installer, and that it produce a theme change and url change for "Finish translations"

Another funny thing. Even very confusing. In fact the second batch process for "Finish translations" running is not the one declared in install.core.inc, it is the batch process called from locale_system_update. This function is called as soon as the update module is enable after submitting the config form hook_modules_installed(), which called with module_invoke_all('modules_installed', $modules_installed); from module_enable.
So to me, using module_enable between two batch import process, will include one more to automatically import .po file when a new module is installed.
Hope that make sense.

@julien: I'm not getting this. Are you still trying to eliminate the second import step, this time by it not being invoked due to only core translations being present? The job of identifying translations files for projects will be with the l10n_update functionality we are integrating in related issues. Not sure you should attempt to re-implement it here for the installer, although point taken that it might not be straightforward to do in the installer and we might need some special case code like this to identify projects so early in the installer.

However, how is this related to fixing the redirection/setup bug?!

There is two things actually. The one related to the redirection/setup bug is only the changes for the module_enable call on the config_form_submit function, because it runs another import batch process when it is called, not the actual files remaining one of the' install steps.
I made some changes related to #13 because on each import process steps, the batch process import all the files, and i reckon it should be change. it's not the actual fix for the issue, you're right.

Updated patch that only run locale batch po import on module_enable if we are not in the installer.

or use

  $request = Request::createFromGlobals();

  if ($request->getBaseUrl() != '/core/install.php') {
    include_once drupal_get_path('module', 'locale') . '/locale.bulk.inc';
    if ($batch = locale_translate_batch_import_files(array(), TRUE)) {
      batch_set($batch);
    }
  }

@julien: thanks for the patch! Is this a "best practice" way to check whether we are in the installer? In Drupal 7, there was an install_task variable that the installer itself checked for completion: http://drupalcode.org/project/drupal.git/blob/refs/heads/7.x:/includes/i...

If something like that exists in Drupal 8, we could do simpler things like

if (variable_get('install_task', '') == 'done')) {
  // do the import
}

(I realize these state variables are moving out to their own system). I have nothing against the proposed solution, but personally don't know if this is best practice now with Symfony or not to check for whether we are in the installer.

Elevating to major since the final install tasks are not run due to this issue.

@Gábor Hojtsy: indeed, each time an install step is completed, we have the last completed task info in the variable $install_state.
Yes as variable_get and variable_set seems, apparently are not the way to go here anymore. We need to code that thing from scratch. I mean this function will not suit as it query the variable table, plus it return "done". I'm not a huge fan of comparing string tbh.
If we could add a server flag variable in function install_begin_request(&$install_state) of install.core.inc, (which is already overriding server variables and also using the symfony $request variable, i reckon it's easier to just set a flag here, make another function on the file that check that flag and return 0 or 1 if the install have not finished for now.
We could be able to do it task by task as the D7 approach.

Thanks @beejeebus on IRC, http://api.drupal.org/api/drupal/core%21includes%21bootstrap.inc/functio... is the simple way to tell if we are in the installer. "Returns TRUE if a Drupal installation is currently being attempted.".

Also could use a couple lines of docs around the condition to mention why are we doing it.

Changed to drupal_installation_attempted

#28: drupal8-installing_drupal_non_English_language_wrong_output-1757078.patch queued for re-testing.

This looks short and sweet now!

A line of documentation would be important here, like "Skip running the translation imports if in the installer, because it would break out of the installer flow. We have built-in support for translation downloads in the installer." (broken down to proper 80 length lines as usual :).

Doc added :)

typo fix, and testbot retry

Looks pretty good and cannot get simpler than this. It fixes a major bug where the last step of the installer is never reached. Also unfortunately not testable in an automated way due to how our simpletest system is set up.

Wow, great catch!

Committed and pushed to 8.x. Thanks!

Congratulations!

awesomeness! :)

Removing from sprint. I don't think this should get a changelog entry or change notice, it is just fixing a bug introduced in D8 :) Thanks all though!