Opened 12 years ago

Closed 11 years ago

#18231 closed Bug (fixed)

i18n javascript_catalog View Pollutes Global Namespace

Reported by: Matthew Tretter <matthew@…> Owned by: nobody
Component: Internationalization Version: dev
Severity: Normal Keywords: javascript i18n
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

In addition to the translation functions (gettext, ngettext, etc.), the global variables catalog and formats are created. These variables are required to exist and be in the correct format in order for the translation functions to work.

Seeing as how these are common words, with a relatively high likelihood of being clobbered, I propose we use the module pattern (an immediately invoked function).

This patch also uses vanilla objects for catalog and formats as those are the preferred type for dictionaries in JavaScript.

Attachments (1)

js-i18n-patch.diff (4.7 KB ) - added by Matthew Tretter <matthew@…> 12 years ago.

Download all attachments as: .zip

Change History (10)

by Matthew Tretter <matthew@…>, 12 years ago

Attachment: js-i18n-patch.diff added

comment:1 by Matthew Tretter <matthew@…>, 12 years ago

The patch corresponds to pull request #13 on the github repository.

https://github.com/django/django/pull/13

comment:2 by Jannis Leidel, 12 years ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Accepting in general, but not generally excited about the indentation changes.

comment:3 by Alexey Boriskin, 12 years ago

What do you think about the following idea: make all the generated javascript to be a string with django template syntax? That would allow us not to worry about indentation changes.

comment:4 by Matthew Tretter, 12 years ago

New commits have been added to the PR on GitHub to use a template.

comment:5 by Aymeric Augustin, 12 years ago

Patch needs improvement: set

The PR no longer applies.

comment:6 by Matthew Tretter, 12 years ago

I've got an updated branch, but I can't update the pull request since the issue is closed. Let me know if want to reopen the PR and have me update it or if I should create a new PR instead.

comment:7 by Ramiro Morales, 12 years ago

Current PR is almost there but is causing one failure and seven errors in the Django test suite, all of the related to the js i18n catalog.

comment:8 by Matthew Tretter, 12 years ago

Woops, sorry about that! I've updated the pull request.

comment:9 by Ramiro Morales <cramm0@…>, 11 years ago

Resolution: fixed
Status: newclosed

In a506b6981bc48caec30bca3de94d2ac3e6fc1660:

Fixed #18231 -- Made JavaScript i18n not pollute global JS namespace.

Also, use Django templating for the dynamic generated JS code and use
more idiomatic coding techniques.

Thanks Matthew Tretter for the report and the patch.

Note: See TracTickets for help on using tickets.
Back to Top