Opened 6 years ago

Closed 20 months ago

Last modified 20 months ago

#10436 closed Bug (fixed)

Application names i18n in the admin app broken

Reported by: ramiro Owned by: ramiro
Component: contrib.admin Version: master
Severity: Normal Keywords: blocktrans trans app_label breadcrumbs capfirst title app-loading
Cc: benspaulding, jezdez, basti@…, someuniquename@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by ramiro)

Having

{% blocktrans with app.name as name %}{{ name }}{% endblocktrans %}

as we currently do in some admin templates shows a decision has been made to allow developers the ability translate application names.

Problem is this feature was DOA (r3270, django/contrib/admin/templates/admin/index.html, similar template code was copied since then to another file: r8474, app_index.html)

See relevant django-users and django-i18n threads for the details.

The attached patch implements as a fix the awkward manual steps needed to workaround this bug outlined by the user patrikk in the first thread, plus the following enhancements:

  • index, app_index: Moved the markup-as-translatable of app names from the template to the view code so the translators aren't forced to translate a second title()'d version of every app name.
  • Consistent use of the capfirst filter on the app names in the breadcrumbs of the different views.
  • Don't touch base_site.html in order to provide for translatability of the app name in the <title> HTML tag for the application-specific model list view. Instead, implement the needed change in the relevant view (app_index) (replacing a "%s administration" msgid with a more translator-friendly "%(app_label)s administration" one in the process). This will spare translators the need to translate both a "<appname>" and a "<appname> administration" literals.
  • Extend above fixes to other templates that also need them (delete_confirm.html, object_history.html)

Notes to reviewers:

Note 1: This patch has nothing to do with the decision of how we will allow the app name metadata to be specified and marked as translatable in the app itself, that seems to be a new feature I'm sure is being dealt with in other tickets. This ticket is much less ambitious: it's about fixing an old internationalization bug in the presentation of that information (irrespective of its origin in the backend) in the admin contrib app.

Note 2: The change_form.html template ran the app label through the escape filter. This has been dropped because it seems inconsistent with what is done in rest of the templates, the app name isn't end user-provided content so the risk of being abused for XSS seems low.

Note 3: There seems to be some inconsistency on how the app name is a) .title()'d for the name that appear in the caption of model tables (index and app_index views) and b) |capfirst'd for use in HTML title and breadcrumbs. This has been left untouched to avoid introducing a backward incompatible change.

Note 4: A further enhancement would be to move presentation-related munging (like calling .title() and/or capfirst() in view code) from views to templates if/when #5972 gets committed but meanwhile this fix can solve this bug now with the tools we have at hand now.

Several of the problems exposed above had already been reported in #9273.

Attachments (7)

10436-app-name-translation-in-admin-r9998.diff (6.3 KB) - added by ramiro 6 years ago.
10436-app-name-translation-in-admin-r10283.diff (8.0 KB) - added by ramiro 6 years ago.
Patch updated to trunk post-bulk-edit and post-r10234
10436-app-name-translation-in-admin-r10314.diff (7.4 KB) - added by ramiro 6 years ago.
Patch with some fixes to the patch iself
10436-app-name-translation-in-admin-r11581.diff (7.9 KB) - added by ramiro 6 years ago.
Patch updated to trunk as of r11581
10436-app-name-translation-in-admin-r12157.diff (7.5 KB) - added by ramiro 6 years ago.
Patch updated to r12157
10436-app-name-translation-in-admin-r12676.diff (6.8 KB) - added by ramiro 5 years ago.
Patch updated to r12676 simplify fixes taing in account fixed made in r12472.
10436-app-name-translation-in-admin-r17377.diff (8.3 KB) - added by ramiro 4 years ago.
Patch updated to trunk as of now

Download all attachments as: .zip

Change History (34)

comment:1 Changed 6 years ago by ramiro

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from Application names i10n in the admin app broken to Application names i18n in the admin app broken

comment:2 Changed 6 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 6 years ago by benspaulding

  • Cc benspaulding added

comment:4 Changed 6 years ago by ramiro

  • Description modified (diff)

(made description more clear)

Changed 6 years ago by ramiro

Patch updated to trunk post-bulk-edit and post-r10234

comment:5 Changed 6 years ago by ramiro

  • Has patch set

Updated patch also extends fixes to two recently added templates: delete_selected_confirmation.html (added in r10121) and change_password.html (added in r10234)

Changed 6 years ago by ramiro

Patch with some fixes to the patch iself

Changed 6 years ago by ramiro

Patch updated to trunk as of r11581

comment:6 Changed 6 years ago by ramiro

  • Description modified (diff)

Updated ticket description.

comment:7 Changed 6 years ago by jezdez

  • Cc jezdez added

Changed 6 years ago by ramiro

Patch updated to r12157

comment:8 Changed 6 years ago by ramiro

  • milestone set to 1.2

comment:9 Changed 5 years ago by ubernostrum

  • milestone changed from 1.2 to 1.3

I know Adrian put that bit into the template, but other than that we've never officially supported translation of app names, and that's really a much larger problem which should be dealt with all in one go rather than piecemeal in tickets like this one. So I'm going to punt it to 1.3.

Changed 5 years ago by ramiro

Patch updated to r12676 simplify fixes taing in account fixed made in r12472.

comment:10 Changed 5 years ago by julien

  • milestone changed from 1.3 to 1.4

I agree with ubernostrum that this is part of a much larger problem. This needs at least a lot more discussion and therefore has now no chance to make it to 1.3.

comment:11 Changed 5 years ago by jezdez

I'm inclined to call this at least related to #3591, worked on in the app-loading GSoC branch.

comment:12 Changed 4 years ago by basti

  • Cc basti@… added

comment:13 Changed 4 years ago by SmileyChris

  • Severity set to Normal
  • Type set to New feature

comment:14 Changed 4 years ago by patchhammer

  • Easy pickings unset
  • Patch needs improvement set

10436-app-name-translation-in-admin-r12676.diff fails to apply cleanly on to trunk

comment:15 Changed 4 years ago by jacob

  • milestone 1.4 deleted

Milestone 1.4 deleted

Changed 4 years ago by ramiro

Patch updated to trunk as of now

comment:16 Changed 4 years ago by ramiro

  • Patch needs improvement unset
  • UI/UX unset

I've attached an update patch for review. I intend to commit it in a couple of days.

comment:17 Changed 4 years ago by ramiro

  • Resolution set to fixed
  • Status changed from new to closed

In [17378]:

Fixed translatability of application names in admin app.

Also, made usage of the filter template tag consistent in some
breadcrumbs elements across the different admin templates.

Fixes #10436.

comment:18 Changed 4 years ago by jezdez

I'm not sure if that's the correct approach, there is no easy way to actually mark an app name for translation, so this would only be a work around at best, an anti-pattern at worst. In other words, I would have vetoed this approach, to be honest, in favor of the app class approach of #3591.

comment:19 Changed 4 years ago by ramiro

In [17389]:

Reverted [17378] until a public API is available for application labels.

This will allow to solve the i18n of such labels in a more holistic way
without the need to introduce temporary, undocumented mechanisms to
translate them.

Refs #3591, #10436

comment:20 Changed 4 years ago by ramiro

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Type changed from New feature to Bug

comment:21 Changed 3 years ago by anonymous

see #15810, similiar aproach using .title which can be translated

comment:22 Changed 3 years ago by paramburu

I've started a discussion about this topic on the Django Developers google group. I would appreciate your opinions on how to keep this moving forward because I believe it has to be implemented some way as soon as possible but I don't want to make the wrong decisions.

https://groups.google.com/forum/?fromgroups=#!topic/django-developers/7f0gKen2ces

Thank you in advance.

comment:23 Changed 2 years ago by aaugustin

  • Status changed from reopened to new

comment:24 Changed 22 months ago by Fak3

  • Cc someuniquename@… added

comment:25 Changed 20 months ago by aaugustin

  • Resolution set to fixed
  • Status changed from new to closed

1.7 contains a proper solution to this issue, based on the app-loading refactor (#3951).

See also b80a8357d683b9a04a14ff46b1a132ad480e8a61.

comment:26 Changed 20 months ago by aaugustin

  • Keywords app-loading added

comment:27 Changed 20 months ago by Aymeric Augustin <aymeric.augustin@…>

In 6e3ca6507cd0a431d63924a708c39f6faa21341b:

Used app_config.verbose_name instead of app_label|capfirst.

An admin view performed the capitalization in the template, unlike all others.

Refs #10436.

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