Hi guys, Didn't dare to post such a big comment to http://code.djangoproject.com/ticket/6409 and the advice I got on #django was to post it here to get some feedback:
First, on my http://code.djangoproject.com/ticket/6409#comment:6comment, all the semicolons in the UA language user preference should be changed to commas. This bug seem to have appeared with the security fix that was committed on r6608 back in October. I've attached a new version of the patch (http://code.djangoproject.com/attachment/ticket/6409/6409-6446-with-regression-tests.diff) with some added regression tests, this is the relevant fragment: -----8x----- >>> from django.utils.translation.trans_real import get_language_from_request >>> g = get_language_from_request >>> from django.http import HttpRequest >>> r = HttpRequest >>> r.COOKIES = {} These tests assumes the es, es_AR, pt and pt_BR translations aren't going to be deleted from the Django source tree. >>> r.META = {'HTTP_ACCEPT_LANGUAGE': 'pt-br'} >>> g(r) 'pt-br' >>> r.META = {'HTTP_ACCEPT_LANGUAGE': 'pt'} >>> g(r) 'pt' >>> r.META = {'HTTP_ACCEPT_LANGUAGE': 'es,de'} >>> g(r) 'es' >>> r.META = {'HTTP_ACCEPT_LANGUAGE': 'es-ar,de'} >>> g(r) 'es-ar' This test assumes there won't be a Django translation to a US variation of the Spanish language, a safe assumption. When the user sets it as the preferred language, the main 'es' translation should be selected instead. >>> r.META = {'HTTP_ACCEPT_LANGUAGE': 'es-us'} >>> g(r) 'es' This tests the following scenario: there isn't a main language (zh) translation of Django but there is a translation to variation (zh_CN) the user sets zh-cn as the preferred language, it should be selected by Django without falling back nor ignoring it. >>> r.META = {'HTTP_ACCEPT_LANGUAGE': 'zh-cn,de'} >>> g(r) 'zh-cn' -----x8----- I'll document the behavior of these tests on three scenarios: a. Before r6608 b. After r6608 c. After r6608 with the patch applied so people smarter than me can decide about this ticket. === a. before r6608 === For example, a checkout of r6607: ====================================================================== FAIL: Doctest: regressiontests.i18n.tests.__test__.misc ---------------------------------------------------------------------- Traceback (most recent call last): File "/mnt/disk2/ramiro/hg-repos/django/svn-r6612/django/test/_doctest.py", line 2169, in runTest raise self.failureException(self.format_failure(new.getvalue())) AssertionError: Failed doctest test for regressiontests.i18n.tests.__test__.misc File "/mnt/disk2/ramiro/hg-repos/django/svn-r6612/tests/regressiontests/i18n/tests.py", line unknown line number, in misc ---------------------------------------------------------------------- File "/mnt/disk2/ramiro/hg-repos/django/svn-r6612/tests/regressiontests/i18n/tests.py", line ?, in regressiontests.i18n.tests.__test__.misc Failed example: g(r) Expected: 'pt-br' Got: 'pt_BR' ---------------------------------------------------------------------- File "/mnt/disk2/ramiro/hg-repos/django/svn-r6612/tests/regressiontests/i18n/tests.py", line ?, in regressiontests.i18n.tests.__test__.misc Failed example: g(r) Expected: 'es-ar' Got: 'es_AR' ---------------------------------------------------------------------- File "/mnt/disk2/ramiro/hg-repos/django/svn-r6612/tests/regressiontests/i18n/tests.py", line ?, in regressiontests.i18n.tests.__test__.misc Failed example: g(r) Expected: 'zh-cn' Got: 'zh_CN' ---------------------------------------------------------------------- Ran 3 tests in 0.239s FAILED (failures=1) The language selections done by the get_language_from_request() function are essentially correct, problem is it returns locale names (xx_YY) and the tests expect language names (xx-yy). More o this later. (Incidentally, reproducing this scenario is a bit involved: One should go to r6608, revert the change to the django/utils/translation/trans_real.py file made on that revision but keep the additions to the testing framework, then the tests/regressiontests/i18n/misc.py file should be created and its contents should be set to be the above tests.) === b. After r6608 === For example, a checkout of r7088: ====================================================================== FAIL: Doctest: regressiontests.i18n.tests.__test__.misc ---------------------------------------------------------------------- Traceback (most recent call last): File "/mnt/disk2/ramiro/hg-repos/django/t6409-base/django/test/_doctest.py", line 2180, in runTest raise self.failureException(self.format_failure(new.getvalue())) AssertionError: Failed doctest test for regressiontests.i18n.tests.__test__.misc File "/mnt/disk2/ramiro/hg-repos/django/t6409-base/tests/regressiontests/i18n/tests.py", line unknown line number, in misc ---------------------------------------------------------------------- File "/mnt/disk2/ramiro/hg-repos/django/t6409-base/tests/regressiontests/i18n/tests.py", line ?, in regressiontests.i18n.tests.__test__.misc Failed example: g(r) Expected: 'pt-br' Got: 'pt' ---------------------------------------------------------------------- File "/mnt/disk2/ramiro/hg-repos/django/t6409-base/tests/regressiontests/i18n/tests.py", line ?, in regressiontests.i18n.tests.__test__.misc Failed example: g(r) Expected: 'es-ar' Got: 'es_AR' ---------------------------------------------------------------------- File "/mnt/disk2/ramiro/hg-repos/django/t6409-base/tests/regressiontests/i18n/tests.py", line ?, in regressiontests.i18n.tests.__test__.misc Failed example: g(r) Expected: 'zh-cn' Got: 'de' ---------------------------------------------------------------------- Ran 3 tests in 0.343s FAILED (failures=1) Note how there is a regression: The tests fail for scenarios that are variations of the one described on the ticket. The get_language_from_request() function keeps returning locale names. (how to reproduce this scenario: the tests/regressiontests/i18n/misc.py file should be patched to add the above tests.) === c. After r6608, applying the patch === For example, a checkout of r7088, apply the entire patch. ---------------------------------------------------------------------- Ran 3 tests in 0.312s OK Now the tests pass, the reported problem is solved (Django works correctly when the user's UA language preference has a xx-yy component). But this also means get_language_from_request() has been changed from returning locale names to return language names. In fact this change one of the things the patch implements. IMVHO the change is correct because when the function gets called from the locale middleware, its return value is passed to the to_locale() function. An additional note: If the patch didn't also take care of #6446 bug, the es-ar test case would fail, that's the reason I included both fixes on the same patch. Regards, -- Ramiro Morales --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~----------~----~----~----~------~----~------~--~---
