Languages get disabled

I can confirm this. When I looked up the database, english got disabled. I tried to enable it but it didn't fix it. Tried to delete the other language and it still didn't fix it. It got fixed after I added another language. Maybe it's the database caching but it's still a mystery what turned it off in the first place.

This is a bug that was present last time I tried 6 dev, about a month ago. I don't know how to fix this.

It happens each time we set another language to be default.

Actually "cannedbrain" gives us a temporary fix. Adding a new language and deleting it will redisplay English.

Here is a more accurate description of the bug:

Settings a new language as the 'default' language on the 'q=admin/settings/language' page causes the previously 'default' language to be disabled.

Shortly I'll explain why this issue is 'critical'.

I'll soon attach a patch.

Steps to reproduce:

1. Look at the attached screenshot. There are four enabled languages there. 'English' is the default language (but it could be any other language).

2. Click the radio button of the 'Spanish' language (or any other) to make this language the default one.

3. Click 'Save configuration'.

4. When the page refreshes you find out that the 'English' language is now disabled (its 'Enabled' checkbox is ticked off.)

Why does this happen?

This is an HTML issue. The 'Enabled' checkbox of the default language is given the 'disabled' HTML attribute (you can see in the screenshot that it's a bit dim). Browsers don't send to the server fields that are disabled (see ref [1]). Since Drupal doesn't find this checkbox in the $_POST data, it thinks that the checkbox is ticked off.

Why am I tagging this as 'critical'?

Because this leads to the 'language_count' variable to get out of sync and eventually to the locale system to get dysfunctional. I used four languages in this demonstration, but had I used only _two_, as is the case in most localized sites, and as the original poster indeed reported, things would have soon turned nasty, because 'language_count' would have reached '1' and when this happens the locale system ignores the 'languages' DB table (that's why the original poster saw the "english drops"). I'm not sure the casual admin would know how to fix her locale system.

Important

Your 'language_count' variable would get out of sync. Don't forget to do variable_set('language_count', n), where 'n' is the number of your enabled languages.

[1] http://www.w3.org/TR/html401/interact/forms.html#h-17.13.2
(That's where the HTML spec says browsers shouldn't send back disabled fields to the server.)

> Here is a more accurate description of the bug:
>
> Settings a new language as the 'default' language on the
> 'q=admin/settings/language' page causes the previously 'default'
> language to be disabled.

Argghhh! I meant: "Setting a new language...", not "settings a ..." (without the "s" :-)

Here's the patch. It's a very simple one: when changing the 'default' language not only the new one is enabled but the old one too (it's already enabled but Drupal erroneously think it isn't: because the browser doesn't post the (disabled) checkbox).

In this patch I also wrapped one increment of 'language_count' in "if ($enabled) {}", as it should be. This isn't directly related to this issue, but since we have already discussed here loosing this variable's sync, I think this is the right place to fix this.

Hm, I don't see why should the *previously enabled* language keep being enabled. It should depend on the checkbox there, it should not be enforced anymore, if it is not the default anymore. Also the $enabled_count should be recorded as 0, if there are no enabled languages (which is practically not possible, as one needs to be the default), so I don't understand why do we need the if ($enabled) either. (Plus there is no variable named $enabled).

Gabor, I'll anwser your two questions shortly. But I think a series of screenshots can do better. I'll post three.

(The first screenshot.)

This screenshot shows our starting point. We have four languages defined on our system. English is the default one. Note that the checkbox of English is dimmed: because this checkbox is disabled; it has the 'disabled' HTML attribute.

(A language which is the default one has its "Enable" checkbox disabled. This is a "usability feature" new to Drupal 6.0.)

(The second screenshot.)

In this screenshot I click the radio button of Spanish, because I want it to become the default language. I'm about to click the 'Save configuration' button.

(The third, last screenshot.)

This screenshot shows what happens after I click the 'Save configuration' button. Drupal announces 'Configuration saved'. Spanish is now the default language.

BUT,

Notice the checkbox of English: it's cleared! English isn't enabled now. That's the bug. (It doesn't involve English specifically; it could have been any other language there.)

This happens because Drupal thinks we cleared this checkbox: because it doesn't see it in $_POST: because browsers don't post fields that are disabled.

>
> Hm, I don't see why should the *previously enabled* language keep
> being enabled. It should depend on the checkbox there,
>

A good question.

Look at the second screenshot (that in comment #10). English is about to be the "previously enabled language" (I'm about to click the 'Save configuration' button). You're asking why this langugae should keep being enabled (during hook_submit). This is because... it's enabled. You see that in the screenshot. It's enabled because the user couldn't disable it: because its "Enable" checkbox is disbaled and the user can't interact with it.

(Of course, another solution to this bug is to remove this ['#attributes']['disabled'] "usability feature". Then Drupal would see this checkbox data in $_POST).

>
> English is about to be the "previously enabled language" (I'm about to click
>

A small error: we need to say "the previously _default_ language". Not the "previously _enabled_ language". I quoted you and didn't notice it wasn't the precise wording.

Now I see your point, but I sill wonder. The screenshots clearly show that the checkbox is *disabled*, not unchecked. It is *checked*, so it should not be possible to modify it, but it should keep being checked when submitted. It would be great to check this in other browsers too, before committing a solution.

>
> Also the $enabled_count should be recorded as 0, if there
> are no enabled languages (which is practically not possible,
> as one needs to be the default), so I don't understand why
> do we need the if ($enabled) either. (Plus there is no variable
> named $enabled).
>

You're talking here about the second hunk in my patch.

My patch contains two hunks.

This second hunk isn't strictly related to the bug described in this issue. Well, actually, it has nothing do to with this bug. Notice that it is indeed applied to a completely different part in the file (a function which is far away: locale_add_language()).

http://api.drupal.org/api/function/locale_add_language/6

You can ignore this hunk for now. I can re-roll this patch without this hunk, if you wish me to.

But this second hunk deals with the same overall problem: keeping the integrity of the 'language_count' variable. I felt it wouldn't be a great sin to include it in this patch as well.

This hunk fixes a problem in locale_add_language(): It is possible to add a language, programatically, which is disabled by default. The locale_add_language() function gets a parameter, $enabled, which tells it whether to enable the newly created language.

The problem is, that this function increments 'language_count' unconditionally: whether the new language is enabled or not. Since 'language_count' is supposed to count the _enabled_ languages, it should only be incremented when this newly created language is enabled. That is, when the $enabled parameter is TRUE. Note that 6 lines above this problematic line you see a conditional which does take $enabled into consideration (as should be!).

> Now I see your point, but I sill wonder. The screenshots
> clearly show that the checkbox is *disabled*, not unchecked.
> It is *checked*, so it should not be possible to modify it, but
>it should keep being checked when submitted. It would be great
> to check this in other browsers too, before committing a solution.

I checked this on Opera and Firefox. Same results.

The HTML spec says that browsers should not send disbaled checkboxes back to the server (I'll expand on this later).

Checboxes have always been problematic in Drupal's FAPI, because there's no way to distinguish between: (1) a checkbox which is on the form and is cleared, and (2) a checkbox which isn't on the form at all.

A checkbox is not like an "input type=text", where if no text was typed into the field there's at least an empty string in $_POST.

All understood. Now I factored the meat of our discussion into comments to the code, so it is understandable later. Finally, committed the attached patch!

Great! thanks! :-)

I would expand the comment:

"(because we will not get the value from that disabled checkox)."

to:

"(because we will not get the value from that disabled checkox --in accordance with the HTML specification, which says that browsers aren't to send disabled controls back to the server.)"

Is it too long? I think this addition to the comment is important, because else people maintaining this code will have no idea why the checkbox isn't seen by Drupal. They would think this is some mystical FAPI issue whereas It has nothing to do with FAPI but with how browsers behave.

(The full explanation, of course, is in the link I posted previously, from W3C's website. That doc talks about "succesful controls" and it rules that "controls that are disabled cannot be successful.")

>
> I would expand the comment: [...]

Gabor, please don't bother with my request. I have more fixes to 'locale.inc' anyway (very minor ones), so I'll include the expansion to this comment in some future patch.