Replace format() placeholders in translatable strings with % formatting

Fixes #5539. Transifex and Django's makemessages command have validation to catch invalid placeholder variables within translated strings - for example, where the translator has translated the variable name - but these only recognise old-style `%` formatting, not the `format` method, and so it's better for us to standardise on % formatting.

To reduce the burden on translators having to re-translate these strings, only the ones using named placeholders (`"Edited page {title}"`) rather than numeric ones (`"Edited page {0}"`) have been changed - hopefully the latter give less room for error.

Also fixed some incorrect use of plurals (verbose_name vs verbose_name_plural) in snippet confirmation messages.

CHANGELOG.txt

@@ -47,6 +47,7 @@ Changelog
  * Fix: Prevent broken images in notification emails when static files are hosted on a remote domain (Eduard Luca)
  * Fix: Replace styleguide example avatar with default image to avoid issues when custom user model is used (Matt Westcott)
  * Fix: `DraftailRichTextArea` is no longer treated as a hidden field by Django's form logic (Sergey Fedoseev)
+ * Fix: Replace format() placeholders in translatable strings with % formatting (Matt Westcott)
 
 
 2.6.2 (19.09.2019)

docs/releases/2.7.rst

@@ -71,6 +71,7 @@ Bug fixes
  * Prevent broken images in notification emails when static files are hosted on a remote domain (Eduard Luca)
  * Replace styleguide example avatar with default image to avoid issues when custom user model is used (Matt Westcott)
  * ``DraftailRichTextArea`` is no longer treated as a hidden field by Django's form logic (Sergey Fedoseev)
+ * Replace format() placeholders in translatable strings with % formatting (Matt Westcott)
 
 
 Upgrade considerations

wagtail/admin/wagtail_hooks.py

@@ -103,14 +103,17 @@ def page_listing_buttons(page, page_perms, is_parent=False):
         yield PageListingButton(
             _('Edit'),
             reverse('wagtailadmin_pages:edit', args=[page.id]),
-            attrs={'aria-label': _("Edit '{title}'").format(title=page.get_admin_display_title())},
+            attrs={'aria-label': _("Edit '%(title)s'") % {'title': page.get_admin_display_title()}},
             priority=10
         )
     if page.has_unpublished_changes:
         yield PageListingButton(
             _('View draft'),
             reverse('wagtailadmin_pages:view_draft', args=[page.id]),
-            attrs={'aria-label': _("Preview draft version of '{title}'").format(title=page.get_admin_display_title()), 'target': '_blank', 'rel': 'noopener noreferrer'},
+            attrs={
+                'aria-label': _("Preview draft version of '%(title)s'") % {'title': page.get_admin_display_title()},
+                'target': '_blank', 'rel': 'noopener noreferrer'
+            },
             priority=20
         )
     if page.live and page.url:
@@ -119,7 +122,7 @@ def page_listing_buttons(page, page_perms, is_parent=False):
             page.url,
             attrs={
                 'target': "_blank", 'rel': 'noopener noreferrer',
-                'aria-label': _("View live version of '{title}'").format(title=page.get_admin_display_title()),
+                'aria-label': _("View live version of '%(title)s'") % {'title': page.get_admin_display_title()},
             },
             priority=30
         )
@@ -129,7 +132,7 @@ def page_listing_buttons(page, page_perms, is_parent=False):
                 _('Add child page'),
                 reverse('wagtailadmin_pages:add_subpage', args=[page.id]),
                 attrs={
-                    'aria-label': _("Add a child page to '{title}' ").format(title=page.get_admin_display_title()),
+                    'aria-label': _("Add a child page to '%(title)s' ") % {'title': page.get_admin_display_title()},
                 },
                 classes={'button', 'button-small', 'bicolor', 'icon', 'white', 'icon-plus'},
                 priority=40
@@ -138,7 +141,7 @@ def page_listing_buttons(page, page_perms, is_parent=False):
             yield PageListingButton(
                 _('Add child page'),
                 reverse('wagtailadmin_pages:add_subpage', args=[page.id]),
-                attrs={'aria-label': _("Add a child page to '{title}' ").format(title=page.get_admin_display_title())},
+                attrs={'aria-label': _("Add a child page to '%(title)s' ") % {'title': page.get_admin_display_title()}},
                 priority=40
             )
 
@@ -148,7 +151,10 @@ def page_listing_buttons(page, page_perms, is_parent=False):
         page=page,
         page_perms=page_perms,
         is_parent=is_parent,
-        attrs={'target': '_blank', 'rel': 'noopener noreferrer', 'title': _("View more options for '{title}'").format(title=page.get_admin_display_title())},
+        attrs={
+            'target': '_blank', 'rel': 'noopener noreferrer',
+            'title': _("View more options for '%(title)s'") % {'title': page.get_admin_display_title()}
+        },
         priority=50
     )
 
@@ -159,35 +165,35 @@ def page_listing_more_buttons(page, page_perms, is_parent=False):
         yield Button(
             _('Move'),
             reverse('wagtailadmin_pages:move', args=[page.id]),
-            attrs={"title": _("Move page '{title}'").format(title=page.get_admin_display_title())},
+            attrs={"title": _("Move page '%(title)s'") % {'title': page.get_admin_display_title()}},
             priority=10
         )
     if page_perms.can_copy():
         yield Button(
             _('Copy'),
             reverse('wagtailadmin_pages:copy', args=[page.id]),
-            attrs={'title': _("Copy page '{title}'").format(title=page.get_admin_display_title())},
+            attrs={'title': _("Copy page '%(title)s'") % {'title': page.get_admin_display_title()}},
             priority=20
         )
     if page_perms.can_delete():
         yield Button(
             _('Delete'),
             reverse('wagtailadmin_pages:delete', args=[page.id]),
-            attrs={'title': _("Delete page '{title}'").format(title=page.get_admin_display_title())},
+            attrs={'title': _("Delete page '%(title)s'") % {'title': page.get_admin_display_title()}},
             priority=30
         )
     if page_perms.can_unpublish():
         yield Button(
             _('Unpublish'),
             reverse('wagtailadmin_pages:unpublish', args=[page.id]),
-            attrs={'title': _("Unpublish page '{title}'").format(title=page.get_admin_display_title())},
+            attrs={'title': _("Unpublish page '%(title)s'") % {'title': page.get_admin_display_title()}},
             priority=40
         )
     if page_perms.can_view_revisions():
         yield Button(
             _('Revisions'),
             reverse('wagtailadmin_pages:revisions_index', args=[page.id]),
-            attrs={'title': _("View revision history for '{title}'").format(title=page.get_admin_display_title())},
+            attrs={'title': _("View revision history for '%(title)s'") % {'title': page.get_admin_display_title()}},
             priority=50
         )
 
@@ -356,7 +362,7 @@ def register_core_features(features):
         'draftail', 'h1', draftail_features.BlockFeature({
             'label': 'H1',
             'type': 'header-one',
-            'description': ugettext('Heading {level}').format(level=1),
+            'description': ugettext('Heading %(level)d') % {'level': 1},
         })
     )
     features.register_converter_rule('contentstate', 'h1', {
@@ -371,7 +377,7 @@ def register_core_features(features):
         'draftail', 'h2', draftail_features.BlockFeature({
             'label': 'H2',
             'type': 'header-two',
-            'description': ugettext('Heading {level}').format(level=2),
+            'description': ugettext('Heading %(level)d') % {'level': 2},
         })
     )
     features.register_converter_rule('contentstate', 'h2', {
@@ -386,7 +392,7 @@ def register_core_features(features):
         'draftail', 'h3', draftail_features.BlockFeature({
             'label': 'H3',
             'type': 'header-three',
-            'description': ugettext('Heading {level}').format(level=3),
+            'description': ugettext('Heading %(level)d') % {'level': 3},
         })
     )
     features.register_converter_rule('contentstate', 'h3', {
@@ -401,7 +407,7 @@ def register_core_features(features):
         'draftail', 'h4', draftail_features.BlockFeature({
             'label': 'H4',
             'type': 'header-four',
-            'description': ugettext('Heading {level}').format(level=4),
+            'description': ugettext('Heading %(level)d') % {'level': 4},
         })
     )
     features.register_converter_rule('contentstate', 'h4', {
@@ -416,7 +422,7 @@ def register_core_features(features):
         'draftail', 'h5', draftail_features.BlockFeature({
             'label': 'H5',
             'type': 'header-five',
-            'description': ugettext('Heading {level}').format(level=5),
+            'description': ugettext('Heading %(level)d') % {'level': 5},
         })
     )
     features.register_converter_rule('contentstate', 'h5', {
@@ -431,7 +437,7 @@ def register_core_features(features):
         'draftail', 'h6', draftail_features.BlockFeature({
             'label': 'H6',
             'type': 'header-six',
-            'description': ugettext('Heading {level}').format(level=6),
+            'description': ugettext('Heading %(level)d') % {'level': 6},
         })
     )
     features.register_converter_rule('contentstate', 'h6', {

wagtail/contrib/modeladmin/views.py

@@ -150,8 +150,9 @@ def get_context_data(self, **kwargs):
         return super().get_context_data(**context)
 
     def get_success_message(self, instance):
-        return _("{model_name} '{instance}' created.").format(
-            model_name=capfirst(self.opts.verbose_name), instance=instance)
+        return _("%(model_name)s '%(instance)s' created.") % {
+            'model_name': capfirst(self.opts.verbose_name), 'instance': instance
+        }
 
     def get_success_message_buttons(self, instance):
         button_url = self.url_helper.get_action_url('edit', quote(instance.pk))
@@ -640,8 +641,9 @@ def get_meta_title(self):
         return _('Editing %s') % self.verbose_name
 
     def get_success_message(self, instance):
-        return _("{model_name} '{instance}' updated.").format(
-            model_name=capfirst(self.verbose_name), instance=instance)
+        return _("%(model_name)s '%(instance)s' updated.") % {
+            'model_name': capfirst(self.verbose_name), 'instance': instance
+        }
 
     def get_context_data(self, **kwargs):
         context = {
@@ -726,8 +728,9 @@ def delete_instance(self):
 
     def post(self, request, *args, **kwargs):
         try:
-            msg = _("{model} '{instance}' deleted.").format(
-                model=self.verbose_name, instance=self.instance)
+            msg = _("%(model_name)s '%(instance)s' deleted.") % {
+                'model_name': self.verbose_name, 'instance': self.instance
+            }
             self.delete_instance()
             messages.success(request, msg)
             return redirect(self.index_url)

wagtail/contrib/settings/views.py

@@ -68,10 +68,10 @@ def edit(request, app_name, model_name, site_pk):
 
             messages.success(
                 request,
-                _("{setting_type} updated.").format(
-                    setting_type=capfirst(setting_type_name),
-                    instance=instance
-                )
+                _("%(setting_type)s updated.") % {
+                    'setting_type': capfirst(setting_type_name),
+                    'instance': instance
+                }
             )
             return redirect('wagtailsettings:edit', app_name, model_name, site.pk)
         else:

wagtail/core/blocks/static_block.py

@@ -12,7 +12,7 @@ class StaticBlock(Block):
     def render_form(self, value, prefix='', errors=None):
         if self.meta.admin_text is None:
             if self.label:
-                return _('{label}: this block has no options.').format(label=self.label)
+                return _('%(label)s: this block has no options.') % {'label': self.label}
             else:
                 return _('This block has no options.')
         return self.meta.admin_text

wagtail/snippets/views/snippets.py

@@ -8,6 +8,7 @@
 from django.urls import reverse
 from django.utils.text import capfirst
 from django.utils.translation import ugettext as _
+from django.utils.translation import ungettext
 
 from wagtail.admin import messages
 from wagtail.admin.auth import permission_denied
@@ -144,10 +145,10 @@ def create(request, app_label, model_name):
 
             messages.success(
                 request,
-                _("{snippet_type} '{instance}' created.").format(
-                    snippet_type=capfirst(model._meta.verbose_name),
-                    instance=instance
-                ),
+                _("%(snippet_type)s '%(instance)s' created.") % {
+                    'snippet_type': capfirst(model._meta.verbose_name),
+                    'instance': instance
+                },
                 buttons=[
                     messages.button(reverse(
                         'wagtailsnippets:edit', args=(app_label, model_name, quote(instance.pk))
@@ -191,10 +192,10 @@ def edit(request, app_label, model_name, pk):
 
             messages.success(
                 request,
-                _("{snippet_type} '{instance}' updated.").format(
-                    snippet_type=capfirst(model._meta.verbose_name_plural),
-                    instance=instance
-                ),
+                _("%(snippet_type)s '%(instance)s' updated.") % {
+                    'snippet_type': capfirst(model._meta.verbose_name),
+                    'instance': instance
+                },
                 buttons=[
                     messages.button(reverse(
                         'wagtailsnippets:edit', args=(app_label, model_name, quote(instance.pk))
@@ -239,15 +240,22 @@ def delete(request, app_label, model_name, pk=None):
             instance.delete()
 
         if count == 1:
-            message_content = _("{snippet_type} '{instance}' deleted.").format(
-                snippet_type=capfirst(model._meta.verbose_name_plural),
-                instance=instance
-            )
+            message_content = _("%(snippet_type)s '%(instance)s' deleted.") % {
+                'snippet_type': capfirst(model._meta.verbose_name),
+                'instance': instance
+            }
         else:
-            message_content = _("{count} {snippet_type} deleted.").format(
-                snippet_type=capfirst(model._meta.verbose_name_plural),
-                count=count
-            )
+            # This message is only used in plural form, but we'll define it with ungettext so that
+            # languages with multiple plural forms can be handled correctly (or, at least, as
+            # correctly as possible within the limitations of verbose_name_plural...)
+            message_content = ungettext(
+                "%(count)d %(snippet_type)s deleted.",
+                "%(count)d %(snippet_type)s deleted.",
+                count
+            ) % {
+                'snippet_type': capfirst(model._meta.verbose_name_plural),
+                'count': count
+            }
 
         messages.success(request, message_content)