Undefined index: langcode in locale_translate_batch_import() and translations not imported

Tagging for sprint and language-ui, reproduction welcome :)

I can confirm I see what LoMo saw. I have not tried the patch yet.

I saw a flash of another error, too quick for a screen shot. I'll check the logs, and I was running a vid recording, so I can check that too later.

side note: the translation directory was already there for me.
and I did this while testing:

831 git checkout 8.x
832 history | grep git
833 git reset --hard
834 git pull --rebase
835 git status
836 rm -r core/modules/language/config/language.detection.yml
837 git status
838 git pull --rebase
839 git reset --hard
840 git status
841 history | grep drush
842 drush sql-drop
...
509 cd ~/foo/d8-git
511 cd sites/default/files/
513 cd translations/
515 curl -O http://ftp.drupal.org/files/translations/7.x/drupal/drupal-7.15.de.po

Summary: I admit, I went through testing like 4 times. I got confused, deleted most of my notes here from being confused... :) (but if people really wanted to see me being confused, there are some vids in my blip.tv of parts of the previous testing of this issue) Finally I had to be very careful to start completely from scratch, and do two sites side by side. [ http://blip.tv/cathytheys/undef-langcode-try4-testing-patch-very-careful... ] I did exactly the same steps and cleared caches occasionally. The difference seems to be that the site with the patch does not have the undef langcode error. (good news!) But the interfaces are translated exactly the same, and they have the same other errors. (kind of bad news: so I didn't see an effect of the patch other than just not seeing the error)

Details:
quick flash of an error (same on site without patch and with patch):

Error importing interface translations
Please continue to -link-the error page-link-
An AJAX HTTP request terminated abnormally. Debugging information follows. Path: /d8-git/batch?id=4&op=do_nojs&op=do StatusText: error ResponseText: ReadyState: 0

Errors in the log (might not have anything to do with this issue/patch as they were there before the patch and also afterward):
Referrer http://localhost:8888/d8-git/core/install.php?langcode=de&profile=standa...
Message Warning: fopen() [function.fopen]: Filename cannot be empty in Drupal\Component\Gettext\PoStreamReader->open() (line 168 of /Users/ctheys/foo/d8-git/core/lib/Drupal/Component/Gettext/PoStreamReader.php).

Referrer http://localhost:8888/d8-git/core/install.php?langcode=de&profile=standa...
Message Warning: ftell() expects parameter 1 to be resource, boolean given in Drupal\Component\Gettext\PoStreamReader->open() (line 169 of /Users/ctheys/foo/d8-git/core/lib/Drupal/Component/Gettext/PoStreamReader.php).

Referrer http://localhost:8888/d8-git/core/install.php?langcode=de&profile=standa...
Message Warning: fgets() expects parameter 1 to be resource, boolean given in Drupal\Component\Gettext\PoStreamReader->readLine() (line 263 of /Users/ctheys/foo/d8-git/core/lib/Drupal/Component/Gettext/PoStreamReader.php).

Site with no patch, after enable module shows the error:

Notice: Undefined index: langcode in locale_translate_batch_import() (line 465 of core/modules/locale/locale.bulk.inc).


With patch: no error there:

Other notes:
without patch: error log:

with patch: error log:
(can see the undef langcode error is gone, but other fopen, etc, errors the same)

And, just to be clear, the sites had the same strings translated in the no patch and patch cases. (but I had to be very careful to do each step the same, with clearing caches, etc.)

no patch:
[edit: ... missed uploading the right file to show the home page on the no patch site. ]

patch:

missed one of the files... here it is attached. I'll edit the previous post to put it in the right spot there.

From just a visual review I suggest to use:

if (empty($options['langcode']) && is_array($matches)) {

instead:

if ((!isset($options['langcode']) || !$options['langcode']) && is_array($matches)) {

I think empty() should just work fine and the line would look more clean.
I hope to be able to give a functional review asap.

I think we should focus writing an automated test for this fix itself and then look at the AJAX error elsewhere. The fix itself looks good :)

In general, where is the priority on this issue? I could focus on something else since the feature freeze is coming soon. Happy to help where ever.

Yeah, the ajax (and maybe the fopen erros) need their own issues. I can open them unless someone beats me to it.

For the automated test, we need to precisely know what to test (which will also help with the manual testing). :) I need to figure out how to write tests, so I'm up for tackling that too, but it'll be a bit slow since it will be my first automatic testing to write.

One of the things I was thinking that might help testing also is to have a 8.x .po file, so it would be more obvious what english is there because they are new strings in 8.x vs just not having been imported because of a bug.

Ideas of stuff to test to see if the translation happened... maybe a log entry?
Or in the admin bar Structure -> Struktur
Or the description of the block module on the modules page ->
Steuert die visuellen Blöcke aus der eine Seite gebaut wurde. Blöcke sind zusätzliche Inhalte, die in bestimmte Bereiche, Regionen einer Webseite angezeigt werden.

Without the patch, the strings do get translated. ... and I think with the patch they dont get translated any earlier in the process.
So I'm trying to find an exact reproducible series of steps that show the problem and show the patch helps with the problem...

here are the steps (the sXX lines are where in the process I took those screen shots, and some lines are just saying what I did in the interface on the site):
1006 git status
1007 chmod -R u+w sites
1008 rm -r sites
1009 checkout sites
1010 git checkout sites
1011 git status
1012 git diff 8.x
1013 git checkout 8.x
1014 git status
1015 git pull --rebase
1016 git reset --hard
1017 git status
1018 git checkout -b undef-2
1021 mkdir sites/default/files
1022 mkdir sites/default/files/translations
1027 curl -O http://ftp.drupal.org/files/translations/7.x/drupal/drupal-7.15.de.po
1030 mv *.po sites/default/files/translations/
1031 chmod a+w sites/default
install site
s01
s02
s03
s04
1032 drush cc all
s05
aggreg and book module page strings in english.
s06
1033 drush cc all
s07
enable book and contact core modules
s08
enable book and content does trigger translating
s09
s09b

I've just created a spin-off issue that takes care about the failed translation import on installation: #1787520: Translations not imported on installation

So this one only needs tests written. Anybody up for that?

@LoMo: looking at the patch, locale_translate_batch_import() looks like it would be used as well when enabling modules (when langcode is not passed either). Is it not reproducible in that case?

Well, there is this issue for translations import in installation: #1787520: Translations not imported on installation. It is not yet resolved. So I'm wondering how come it works for you. Maybe posting some feedback there would be great.

Ok, looks like you cannot reproduce anymore, so setting this accordingly. Please reopen if you can. I have not heard about people with similar problems.

Looks like this issue can probably be closed too.

But I'll see (hopefully this evening) if I can reproduce any of my initial experience or what they describe in the (similar sounding) issue.

That was seven years ago :-) We should have badges for these :p

Regards to you all!