Opened 8 years ago

Closed 7 years ago

#18231 closed Bug (fixed)

i18n javascript_catalog View Pollutes Global Namespace

Reported by: Matthew Tretter <matthew@…> Owned by: nobody
Component: Internationalization Version: master
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


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@…> 8 years ago.

Download all attachments as: .zip

Change History (10)

Changed 8 years ago by Matthew Tretter <matthew@…>

Attachment: js-i18n-patch.diff added

comment:1 Changed 8 years ago by Matthew Tretter <matthew@…>

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

comment:2 Changed 8 years ago by Jannis Leidel

Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

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

comment:3 Changed 8 years ago by Alexey Boriskin

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 Changed 8 years ago by Matthew Tretter

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

comment:5 Changed 8 years ago by Aymeric Augustin

Patch needs improvement: set

The PR no longer applies.

comment:6 Changed 8 years ago by Matthew Tretter

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 Changed 8 years ago by Ramiro Morales

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 Changed 8 years ago by Matthew Tretter

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

comment:9 Changed 7 years ago by Ramiro Morales <cramm0@…>

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