Regression: early installer is not in RTL after selecting RTL language

tagging and adding contributor task documents to the issue summary

in case someone wants to use a brute force way of finding which commit broke this:
webchick's how to post http://webchick.net/node/99

sorry, without specifics, this isn't novice yet. updating issue summary remaining tasks.

also tagging front end so our front end focus board has this. (http://www.drupal8multilingual.org/issues/frontend)

Using git bisect shows that this got broken when the monster patch from #1862202: Objectify the language system landed.

I'll write tests and attempt to fix the issue over the next couple of days.

thanks, adding that as the parent of this issue.

Proper issue title.

I'm also ambivalently surprised that your bisect yielded the language objectification issue. I could have sworn that I'm guilty ;-)

I initially also thought #2213357: Use a proper kernel in early installer was the cause, but I double-checked and it was broken prior to that commit. Can you find solace in the fact that if it wouldn't have been broken, your change would probably have broken it? ;)

Here's a test for this (expecting it to fail), now trying to fix the actual issue...

So I'm able to "fix" this by adding something like:

    // Set the default language to the language selected in the installer.
    $container
      ->register('language.default', 'Drupal\Core\Installer\LanguageDefault')
      ->addArgument(array('id' => <chosen language here>));

to InstallerServiceProvider::register(). However, this seems to not be right approach, since it adds a dependency on a global (the selected language). I also tinkered with a "InstallerDefaultLanguage" class and moved the global there, but it still feels icky.

(removed frontend tag since this is a backend issue)

Prior to appying the patch:

After applying the patch:

(unassigning myself since I will be AFK for a week or so, don't want to hold up progress on this issue)

I wonder if we should have the opposite test also, that the installer stays in ltr, with a ltr language.

--
this is adding docs like @file for the file and docs on methods. see interdiff.

Here's a new patch with LTR language direction assertion in the German translation installer test.

So IMHO it would be better to use something else to avoid confusion. Is the global value the best way to pass on this info?

Yes, the global is ugly, but I couldn't think of a better, cleaner way to implement this.

However, I assumed LanguageDefault was an immutable object. Turns out it isn't, so we can just do \Drupal::service('language.default')->set(), making this patch both cleaner and simpler.

Reviewing this latest, 2240007-17-installer-rtl.patch

Before applying the patch:

After applying the patch

Works. It would be perfect if the translation was also correct in step 2, but it may be another issue.

I tested the latest patch 2240007-17, it works fine. The translation in step two still needs work, but that may be another issue.

Sorry for not getting back to this earlier.

Patch looks good to me; just some minor cleanups.

(Note that I have renames=copies enabled; attached patch still creates the new test; git merely detected that the new file is a copy.)

Nice catch!

Can you find solace in the fact that if it wouldn't have been broken, your change would probably have broken it? ;)

LOL :)

Committed and pushed to 8.x. Thanks!