Currently the "language neutral" concept is used in two ways:

  • to indicate the language of nodes which have not been explictly assigned a language;
  • to indicate the language of fields which have not been explictly assigned a language;

In both cases the "language neutral" concept has the same meaning, but the actual values assigned are different: nodes use an empty string, fields a ISO639-2 language code (zxx) which means No linguistic content / Not applicable.

Having two different language codes to indicate the same language is both wrong and error prone, AAMOF Field API has to take into account this situation, subverting the usual dependency order which sees Node module depending on the Field module.

I'd suggest to refactor Node to make it use the standard language code.

CommentFileSizeAuthor
#30 language_none-635094-30.patch2.25 KBplach
Repository checkout: failed to checkout from [:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal/]. View
#27 language_none-635094-25.patch672 bytesplach
Failed to install Drupal on MySQL 5.0 InnoDB. View
#16 language_none-635094-16.patch156.68 KBplach
Passed on all environments. View
#16 language_none-635094-work.patch21.25 KBplach
Passed on all environments. View
#14 language_none-635094-work.patch21.25 KBplach
Invalid PHP syntax in system.install. View
#14 language_none-635094-14.patch156.76 KBplach
Invalid PHP syntax in system.install. View
#11 language_none-635094-11.patch154.56 KBplach
Unable to apply patch language_none-635094-11.patch View
#8 language_none-635094-8.patch153.88 KBplach
Passed on all environments. View
#7 language_none-635094-7.patch19.55 KBplach
Passed on all environments. View
#5 language_none-635094-5.patch10.13 KBplach
Failed on MySQL 5.0 ISAM, with: 14,911 pass(es), 2 fail(s), and 0 exception(es). View
#1 language_none-635094-1.patch142.83 KBplach
Failed on MySQL 5.0 ISAM, with: 14,915 pass(es), 0 fail(s), and 75 exception(es). View
#1 language_none-635094-work.patch8.09 KBplach
Failed on MySQL 5.0 ISAM, with: 14,914 pass(es), 0 fail(s), and 75 exception(es). View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

plach’s picture

Status: Active » Needs review
FileSize
8.09 KB
Failed on MySQL 5.0 ISAM, with: 14,914 pass(es), 0 fail(s), and 75 exception(es). View
142.83 KB
Failed on MySQL 5.0 ISAM, with: 14,915 pass(es), 0 fail(s), and 75 exception(es). View

The current patch moves the FIELD_LANGUAGE_NONE definition to bootstrap.inc, rename it to LANGUAGE_NONE and make Node use it instead of the empty string.

Attached is also a "light" version of the patch which does not rename FIELD_LANGUAGE_NONE. You can use this one for reviews, as the real one is bloated by hundreds of braindead search/replace hunks.

We just need an upgrade path and then we should be ok.

plach’s picture

Issue tags: +API clean-up
yched’s picture

Makes sense IMO, and +1 for a shorter constant :-).
Patch raises exceptions, though.

Status: Needs review » Needs work

The last submitted patch failed testing.

plach’s picture

Status: Needs work » Needs review
FileSize
10.13 KB
Failed on MySQL 5.0 ISAM, with: 14,911 pass(es), 2 fail(s), and 0 exception(es). View

This one provides the upgrade path and should fix tests. I removed FIELD_LANGUAGE_NONE renaming, but I'll add it again as soon as I get an ok for the light version.

@yched: ;)

Status: Needs review » Needs work

The last submitted patch failed testing.

plach’s picture

Status: Needs work » Needs review
FileSize
19.55 KB
Passed on all environments. View

Obviously URL aliases must be fixed too.

plach’s picture

FileSize
153.88 KB
Passed on all environments. View

DamZ reviewed #7 in IRC and asked for the Big One!

Damien Tournoud’s picture

Status: Needs review » Reviewed & tested by the community

I like it, and I hope the test bot will too.

Dries’s picture

I'm OK with this but I do want to note that, with this patch, we start mixing ISO-639-1 (two character code) and ISO-639-2 (three character codes). That is probably not a crime, but might be somewhat unexpected.

plach’s picture

FileSize
154.56 KB
Unable to apply patch language_none-635094-11.patch View

Rereolled #8 and added the following hunk as #624290: Wrong default comment language went in:

-    '#value' => isset($comment->language) ? $comment->language : '',
+    '#value' => isset($comment->language) ? $comment->language : LANGUAGE_NONE,

@Dries:

I'm OK with this but I do want to note that, with this patch, we start mixing ISO-639-1 (two character code) and ISO-639-2 (three character codes). That is probably not a crime, but might be somewhat unexpected.

I'd prefer to go with a ISO-639-1 language code too, unfortunately there is nothing like zxx in it.

OTOH the w3c specification of language tags allows mixing 2 and 3 character codes, AAMOF only languages defined in the IANA registry are legal and "When languages have both an ISO 639-1 two-character code and a three-character code (assigned by ISO 639-2, ISO 639-3, or ISO 639-5), only the ISO 639-1 two-character code is defined in the IANA registry."

Hence the IANA registry actually mixes 2 and 3 language codes, as we are doing here for the lack of an appropriate 2 characters code.

Status: Reviewed & tested by the community » Needs review
Issue tags: -translatable fields, -API clean-up

Dries requested that failed test be re-tested.

Status: Needs review » Needs work
Issue tags: +translatable fields, +API clean-up

The last submitted patch failed testing.

plach’s picture

Status: Needs work » Needs review
FileSize
156.76 KB
Invalid PHP syntax in system.install. View
21.25 KB
Invalid PHP syntax in system.install. View

rerolled

Status: Needs review » Needs work

The last submitted patch failed testing.

plach’s picture

FileSize
21.25 KB
Passed on all environments. View
156.68 KB
Passed on all environments. View

system updates count went ahead, meanwhile

plach’s picture

Status: Needs work » Needs review
plach’s picture

Status: Needs review » Reviewed & tested by the community

Green. The patch to be committed is language_none-635094-16.patch (the big one), the other is just for reviewers' convenience.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

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

anarchivist’s picture

Status: Closed (fixed) » Active

Actually, I'm a little concerned about this - zxx in my opinion is not the proper code for this. See this FAQ entry on the Library of Congress's ISO 639-2 FAQ. By specifying zxx as the code, Drupal will be saying that nodes don't have any linguistic content! While in some cases this might be true, it's a pretty safe assumption that most Drupal nodes everywhere contain some linguistic content. und would probably be more correct.

plach’s picture

I didn't notice the und code, I agree that it would be more appropriate, and the replacement would be rather easy.

pwolanin’s picture

The 'und' code has been there all along in the spec - I think there are other issues where it was argued against. I don't really care, but seems like a bikeshed argument at this point.

Damien Tournoud’s picture

I haven't noticed 'und' either. Sounds like a good idea. I'm not aware of any discussions between zxx and und, Peter do you have specific pointers?

plach’s picture

Status: Active » Needs review

I'd like to read that discussion too. Meanwhile here's a patch that replaces zxx with und.

plach’s picture

Issue tags: -translatable fields

This does not concerns TF anymore.

plach’s picture

FileSize
672 bytes
Failed to install Drupal on MySQL 5.0 InnoDB. View

the patch

Status: Needs review » Needs work

The last submitted patch failed testing.

pwolanin’s picture

Hmm, maybe I'm misremembering - all I can find is http://drupal.org/node/394728#comment-2303062 where I suggested 'und' would be better :)

plach’s picture

Status: Needs work » Needs review
FileSize
2.25 KB
Repository checkout: failed to checkout from [:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal/]. View

Can't reproduce any error on install but I found a couple of tests using explicitly 'zxx' instead of LANGUAGE_NONE.

@pwolanin: I didn't read any particular argument against 'und' in that issue. It seems we can safely go on, right?

pwolanin’s picture

@plach - I will be annoyed to have to fix up Apache Solr module, but otherwise, 'und' seems fine to me.

Dries’s picture

Status: Needs review » Fixed

Committed to CVS HEAD.

catch’s picture

Status: Fixed » Needs work
Issue tags: +Needs Documentation

This needs upgrade docs. I just cvs-upped and got notices everywhere. We don't do HEAD-HEAD upgrades yet, but it took me a good ten minutes to work out there was no typo for uid anywhere and find this issue, it's quite likely some modules other than ApacheSolr are depending on the old field structure too.

plach’s picture

@catch: which should be the proper place to document this?

catch’s picture

axyjo’s picture

Status: Needs work » Needs review
catch’s picture

Status: Needs review » Fixed

Looks great.

Status: Fixed » Closed (fixed)

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

Andrew Schulman’s picture