Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #9321 -- Deprecated hard-coding of help text in model ManyToMan…

…yField fields.

This is backward incompatible for custom form field/widgets that rely
on the hard-coded 'Hold down "Control", or "Command" on a Mac, to select
more than one.' sentence.

Application that use standard model form fields and widgets aren't
affected but need to start handling these help texts by themselves
before Django 1.8.

For more details, see the related release notes and deprecation timeline
sections added with this commit.
  • Loading branch information...
commit 4ba1c2e785feecfa7a47aa5336a2b595f086a765 1 parent 2fd6128
Ramiro Morales ramiro authored
5 django/db/models/fields/related.py
View
@@ -11,7 +11,7 @@
from django.utils.encoding import smart_text
from django.utils import six
from django.utils.deprecation import RenameMethodsBase
-from django.utils.translation import ugettext_lazy as _, string_concat
+from django.utils.translation import ugettext_lazy as _
from django.utils.functional import curry, cached_property
from django.core import exceptions
from django import forms
@@ -1348,9 +1348,6 @@ def __init__(self, to, db_constraint=True, **kwargs):
super(ManyToManyField, self).__init__(**kwargs)
- msg = _('Hold down "Control", or "Command" on a Mac, to select more than one.')
- self.help_text = string_concat(self.help_text, ' ', msg)
-
def _get_path_info(self, direct=False):
"""
Called by both direct an indirect m2m traversal.
5 django/forms/models.py
View
@@ -18,7 +18,7 @@
from django.utils.datastructures import SortedDict
from django.utils import six
from django.utils.text import get_text_list, capfirst
-from django.utils.translation import ugettext_lazy as _, ugettext
+from django.utils.translation import ugettext_lazy as _, ugettext, string_concat
__all__ = (
@@ -1104,6 +1104,9 @@ def __init__(self, queryset, cache_choices=False, required=True,
super(ModelMultipleChoiceField, self).__init__(queryset, None,
cache_choices, required, widget, label, initial, help_text,
*args, **kwargs)
+ if isinstance(self.widget, SelectMultiple):
+ msg = _('Hold down "Control", or "Command" on a Mac, to select more than one.')
+ self.help_text = string_concat(self.help_text, ' ', msg) if self.help_text else msg
Donald Stufft Collaborator
dstufft added a note

This has broken things for me in a project I've been running against Django master.

I believe that if self.help_text is a lazy translation than if self.help_text here forces the translation to happen for real. Commenting out the if self.help_text else msg fixes the error.

Marc Tamlyn Collaborator

Does changing it to a self.help_text is not None solve it?

Donald Stufft Collaborator
dstufft added a note

I'm not sure if self.help_text is not None will test true/false in the proper circumstances but it fixes the problem where this forces a translated self.help_text to resolve itself at import time.

Ramiro Morales Collaborator
ramiro added a note

@dstufft What are the symptoms of the breakage?. I will work on fixing that together with some refinements I want to perform and I'd like to write a test case.

@mjtamlyn I'm afraid Donald is right.

In retrospect it would have been nice is the default value of he help_text arguments at a couple of levels would have been None. Then we could do that somewhat arbitray appending of the hard-coded string in that default case and not do it in the case the user explicitly set it to any non-None values (e.g. '' for "I don't need no stinkin' help text"). But alas...

Donald Stufft Collaborator
dstufft added a note

@ramiro I get errors trying to import things because in another app I imported from django.contrib.auth.forms.

https://gist.github.com/dstufft/f156c75384d33f1d3ffb has tracebacks from my test run suite.

As far as I can tell it's a circular/recursive import. When loading the apps my app imports from django.contrib.auth.forms which triggers the import time resolution of the translation, which requires importing/loading all of the apps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
def clean(self, value):
if self.required and not value:
5 docs/internals/deprecation.txt
View
@@ -392,6 +392,11 @@ these changes.
* The ``CACHE_MIDDLEWARE_ANONYMOUS_ONLY`` setting will be removed.
+* Usage of the hard-coded *Hold down "Control", or "Command" on a Mac, to select
+ more than one.* string to override or append to user-provided ``help_text`` in
+ forms for ManyToMany model fields will not be performed by Django anymore
+ either at the model or forms layer.
+
2.0
---
51 docs/releases/1.6.txt
View
@@ -481,6 +481,44 @@ parameters. For example::
``SQLite`` users need to check and update such queries.
+.. _m2m-help_text:
+
+Help text of model form fields for ManyToManyField fields
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+HTML rendering of model form fields corresponding to
+:class:`~django.db.models.ManyToManyField` ORM model fields used to get the
+hard-coded sentence
+
+ *Hold down "Control", or "Command" on a Mac, to select more than one.*
+
+(or its translation to the active locale) imposed as the help legend shown along
+them if neither :attr:`model <django.db.models.Field.help_text>` nor :attr:`form
+<django.forms.Field.help_text>` ``help_text`` attribute was specified by the
+user (or appended to, if ``help_text`` was provided.)
+
+This happened always, possibly even with form fields implementing user
+interactions that don't involve a keyboard and/or a mouse and was handled at the
+model field layer.
+
+Starting with Django 1.6 this doesn't happen anymore.
+
+The change can affect you in a backward incompatible way if you employ custom
+model form fields and/or widgets for ``ManyToManyField`` model fields whose UIs
+do rely on the automatic provision of the mentioned hard-coded sentence. These
+form field implementations need to adapt to the new scenario by providing their
+own handling of the ``help_text`` attribute.
+
+Applications that use Django :doc:`model form </topics/forms/modelforms>`
+facilities together with Django built-in form :doc:`fields </ref/forms/fields>`
+and :doc:`widgets </ref/forms/widgets>` aren't affected but need to be aware of
+what's described in :ref:`m2m-help_text-deprecation` below.
+
+This is because, as an temporary backward-compatible provision, the described
+non-standard behavior has been preserved but moved to the model form field layer
+and occurs only when the associated widget is
+:class:`~django.forms.SelectMultiple` or a subclass.
+
Miscellaneous
~~~~~~~~~~~~~
@@ -707,3 +745,16 @@ you can set set the ``form_class`` attribute to a ``ModelForm`` that explicitly
defines the fields to be used. Defining an ``UpdateView`` or ``CreateView``
subclass to be used with a model but without an explicit list of fields is
deprecated.
+
+.. _m2m-help_text-deprecation:
+
+Munging of help text of model form fields for ManyToManyField fields
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+All special handling of the ``help_text`` attibute of ManyToManyField model
+fields performed by standard model or model form fields as described in
+:ref:`m2m-help_text` above is deprecated and will be removed in Django 1.8.
+
+Help text of these fields will need to be handled either by applications, custom
+form fields or widgets, just like happens with the rest of the model field
+types.
Donald Stufft

This has broken things for me in a project I've been running against Django master.

I believe that if self.help_text is a lazy translation than if self.help_text here forces the translation to happen for real. Commenting out the if self.help_text else msg fixes the error.

Marc Tamlyn

Does changing it to a self.help_text is not None solve it?

Donald Stufft

I'm not sure if self.help_text is not None will test true/false in the proper circumstances but it fixes the problem where this forces a translated self.help_text to resolve itself at import time.

Ramiro Morales

@dstufft What are the symptoms of the breakage?. I will work on fixing that together with some refinements I want to perform and I'd like to write a test case.

@mjtamlyn I'm afraid Donald is right.

In retrospect it would have been nice is the default value of he help_text arguments at a couple of levels would have been None. Then we could do that somewhat arbitray appending of the hard-coded string in that default case and not do it in the case the user explicitly set it to any non-None values (e.g. '' for "I don't need no stinkin' help text"). But alas...

Donald Stufft

@ramiro I get errors trying to import things because in another app I imported from django.contrib.auth.forms.

https://gist.github.com/dstufft/f156c75384d33f1d3ffb has tracebacks from my test run suite.

As far as I can tell it's a circular/recursive import. When loading the apps my app imports from django.contrib.auth.forms which triggers the import time resolution of the translation, which requires importing/loading all of the apps.

Please sign in to comment.
Something went wrong with that request. Please try again.