To reproduce:

  1. Enable locale module
  2. Configure the site with more that one language
  3. Set 'language negotiation' with 'path prefix'
  4. Disable a language
  5. The disabled language paths are still valid

This patch checks the state of the language part of the url.

CommentFileSizeAuthor
#8 enabled_language_d.patch2.23 KBtheborg
#6 enabled_language_c.patch1.7 KBtheborg
#2 enabled_language_b.patch1.52 KBtheborg
enabled_language_a.patch806 bytestheborg

Comments

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work

Well, this looks like a more widespread bug here. All of language_initialize() uses the complete language list, not only the enabled languages. We should have a common $languages = language_list('enabled'); $languages = $languages[1]; at the top and use that IMHO.

theborg’s picture

theborg commented
Status: Needs review » Needs work
StatusFileSize
new enabled_language_b.patch1.52 KB

After some tests I agree with Gábor comments that all of language_initialize() should use only the enabled languages.

Patch reformed with $languages = language_list('enabled'); $languages = $languages[1];.

Also note that after removing language_list('prefix'); from 'path prefix mode' the paths with the default language (en) on the url are also valid:

Before:
/node: valid path
After:
/node and /en/node :valid paths
theborg’s picture

theborg commented
Status: Needs work » Needs review
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

This looks buggy, because the array used in the LANGUAGE_NEGOTIATION_PATH* cases is indexed with the path prefix and not the language code. If I apply your patch, the custom path prefix feature goes away, and it is only possible to use the language code as path prefix.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

I mean it is indexed by path prefix before the patch but indexed by language code after the patch.

theborg’s picture

theborg commented
Status: Needs work » Needs review
StatusFileSize
new enabled_language_c.patch1.7 KB

Ay!, didn't notice the custom path prefix feature, sorry.

With reordered $language array by prefix.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work

Well, I'd rather do foreach on the language list as in LANGUAGE_NEGOTIATION_DOMAIN and return when found instead of building up a temporary array. Also renaming the $language temp variable in LANGUAGE_NEGOTIATION_PATH* to $prefix makes a lot of sense. Don't know why we did not do that initially.

theborg’s picture

theborg commented
Status: Needs work » Needs review
StatusFileSize
new enabled_language_d.patch2.23 KB

Added foreach block.
Using $prefix as variable.

Thanks Gábor.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Fixed

Looks much better. I committed this with one extra line of comment added in the $mode == LANGUAGE_NEGOTIATION_PATH_DEFAULT case. Thanks!

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.