* severity: => Normal
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
* easy: => 0
* needs_docs: 1 => 0
* type: => Bug
Comment:
The attached patch solves two tickets at once, i.e. this one and #14402.
The reason is that the bug fixes for those two tickets both rely on the
introduction of the same new method `Widget.alter_help_text`. If this
approach is validated then I'll break the patch in two, and #14402 can be
checked in after this one.
The idea behing `alter_help_text` is to allow the widget to customize the
form field's help text -- for example `SelectMultiple` would now be
responsible for appending the '`Hold down "Control"`' message instead of
letting `ManyToManyField` do it. This new feature introduces a backwards
incompatible change, although the upgrade path is really simple. See the
doc in the patch for more info about that.
Let me know what you think!
--
Ticket URL: <http://code.djangoproject.com/ticket/9321#comment:14>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
* needs_better_patch: 0 => 1
Comment:
Yeah, I really think this callback API is odd, I don't like it at all, why
not just have a `extra_help_text` class attribute in a widget that is
appended if it exists?
--
Ticket URL: <http://code.djangoproject.com/ticket/9321#comment:15>
Comment (by julien):
Well, this callback would allow the widget to operate more changes that
just append some text, for example prepend some text or replace it
altogether. A bit like the way the `ModelAdmin.save_model()` hook works.
If you think that's a YAGNI and that we should only allow appending text,
then a `extra_help_text` class attribute would work. Note that it would
also be backwards compatible (which is cool, I think, if we want to get it
right).
--
Ticket URL: <http://code.djangoproject.com/ticket/9321#comment:16>
Comment (by julien):
Just following on your comment, I see your point about the oddness of that
callback. It felt similarly odd to me when the `ModelAdmin.save_model()`
and `ModelAdmin.delete_model()` hooks were introduced. I think the oddness
is the price to pay for more flexibility and future-proofness. So I'm a
bit ambivalent about this. I'll resubmit a patch using the
`extra_help_text` class attribute so we can compare approaches.
--
Ticket URL: <http://code.djangoproject.com/ticket/9321#comment:17>
Comment (by SmileyChris):
I also don't like the fact this is adding the default help text for all
`SelectMultiple` widgets, as noted by the requirement to change current
tests.
--
Ticket URL: <http://code.djangoproject.com/ticket/9321#comment:18>
Comment (by julien):
@SmileyChris: Yes, this is a manifestation of the backwards incompatible
change of this patch. The "Hold down Control" message only makes sense
when used with a `<select>` widget where multiple items can be selected,
i.e. with `SelectMultiple`. So, had the behaviour been proper, then that
message should have always been there in the tests, not just when used in
the context of a `ManyToManyField`. I see this change more as way of re-
establishing the correct behaviour. What do you think?
--
Ticket URL: <http://code.djangoproject.com/ticket/9321#comment:19>
Comment (by SmileyChris):
I'd prefer to minimize the backwards compatibility of the patch. I don't
really want people to have to hunt through and modify every instance of
`SelectMultiple` they've used to continue to have their help_text
unmolested.
--
Ticket URL: <http://code.djangoproject.com/ticket/9321#comment:20>
Comment (by SmileyChris):
Er, minimize the backwards ''in''compatibility...
--
Ticket URL: <http://code.djangoproject.com/ticket/9321#comment:21>
Comment (by julien):
On further reflection, one radical approach would be to remove the 'Hold
down Control' extra message altogether, and let the developers directly
add some extra information, if they see fit, using the traditional way via
the model field or form field's `help_text` attribute. The use of that
extra message is actually quite debatable, and it has caused quite some
confusion for years from an implementation standpoint.
Anyways, just a thought.
--
Ticket URL: <http://code.djangoproject.com/ticket/9321#comment:22>
Comment (by julien):
OK, thank you all for your feedback. I'll try to summarize the problem and
discussions so far. So, an **ideal** solution would:
* stop `ManyToManyField` from directly appending the 'Hold down Control'
message, since this has nothing to do with ORM-level business.
* allow **non-standard** widgets and widgets **inheriting** from
`SelectMultiple` to **not** append the 'Hold down Control' message when
used in conjunction with a `ManyToManyField`.
* be backwards compatible as it would potentially affect a lot of people:
* When the **default** `SelectMultiple` widget, or any widget
**inheriting** from `SelectMultiple`, **is not** used in conjunction with
a `ManyToManyField`, then **do not** append the message.
* When the **default** `SelectMultiple` **is** used in conjunction with
a `ManyToManyField`, then **do** append the message.
A compromise needs to be found between doing things right and not annoying
too many people with a potentially backwards incompatible change. The
challenge is that, as much as `ManyToManyField` should not be allowed to
guess which widget will be used, the `SelectMultiple` widget should also
not trying to guess that it's used with a `ManyToManyField`. Maybe the
compromise lies in the form field's realm, in particular
`ModelMultipleChoiceField`.
Finally, a bit of forensic research reveals this behaviour was introduced
at least at the time of the magic-removal branch merge:
http://code.djangoproject.com/browser/django/trunk/django/db/models/fields/related.py?rev=2809#L580
--
Ticket URL: <http://code.djangoproject.com/ticket/9321#comment:23>
Comment (by jezdez):
Replying to [comment:17 julien]:
> Just following on your comment, I see your point about the oddness of
that callback. It felt similarly odd to me when the
`ModelAdmin.save_model()` and `ModelAdmin.delete_model()` hooks were
introduced. I think the oddness is the price to pay for more flexibility
and future-proofness. So I'm a bit ambivalent about this. I'll resubmit a
patch using the `extra_help_text` class attribute so we can compare
approaches.
Honestly this is not a good comparison at all, `ModelAdmin` is at a
completely different abstraction layer; users are encouraged to write them
while Widgets (especially the SelectMultiple) not so much.
--
Ticket URL: <http://code.djangoproject.com/ticket/9321#comment:24>
Comment (by julien):
Replying to [comment:24 jezdez]:
> Honestly this is not a good comparison at all, `ModelAdmin` is at a
completely different abstraction layer; users are encouraged to write them
while Widgets (especially the SelectMultiple) not so much.
The comparison was just purely about the hook implementation paradigm,
nothing more. I've actually had to write custom widgets on many occasions
for the admin (in particular for the `ManyToManyField`), which is why I've
always been keen to fix this ticket :-)
--
Ticket URL: <http://code.djangoproject.com/ticket/9321#comment:25>
* ui_ux: => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:26>
Comment (by anonymous):
#16370 reported this again.
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:27>
Comment (by anonymous):
I just ran into this and am having trouble finding a workaround. Can
someone list the preferred workaround until the bug is resolved?
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:28>
* cc: carlos.palol@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:30>
Comment (by anonymous):
I solved this with:
class mymodelform(ModelForm):
class Meta:
pass
def __init__(self, *args, **kwargs):
super(RegisterForm, self).__init__(*args, **kwargs)
self.fields['staying'].help_text = ''
self.fields['family_members'].help_text = ''
don't know if this is 'kosher' but it seems to work for me!
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:31>
Comment (by dblado):
and w/ formatting and me logged in
I solved this with:
{{{#!python
class mymodelform(ModelForm):
class Meta:
pass
def __init__(self, *args, **kwargs):
super(RegisterForm, self).__init__(*args, **kwargs)
self.fields['staying'].help_text = ''
self.fields['family_members'].help_text = ''
}}}
don't know if this is 'kosher' but it seems to work for me!
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:32>
* status: reopened => closed
* version: 1.2 => 1.4
* resolution: => fixed
* needs_tests: 0 => 1
Comment:
Please accept my apologies for this patch if the format is wrong or
something similar.
I think I've fixed this bug in a fairly simple way.
The thing that is at issue here is the addition of widget-related help-
text by the Database field object (ManyToManyField) instead of the Form
field object (MultipleChoiceField, ModelMultipleChoiceField,
TypedChoiceField etc.)
The patch does the following :
* remove the 2 lines that add the message in
django.db.models.related.ManyToManyField.__init__
* add a private method to django.forms.fields.ChoiceField field called
_patch_help_text().
* add calls to this method in the __init__ of MultipleChoiceField and
ModelMultipleChoiceField
The _patch_help_text() method, when called :
* checks the widget on the form and updates the help text if the widget is
a SelectMultiple.
* uses type() to check self.widget so the message isn't printed for
subclasses of SelectMultiple
What do u think? Sorry, I haven't read up on writing tests yet, so no
tests. But, the goal was simple, show the message for SelectMultiple,
suppress it all other times.
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:33>
* status: closed => reopened
* resolution: fixed =>
Comment:
A ticket is closed when the patch is applied, not when the patch is
provided. Reopening.
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:34>
* cc: domen@… (added)
Comment:
What else is needed for this one to apply?
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:35>
* cc: dguardiola@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:36>
* cc: jpic (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:37>
* cc: ivan_virabyan (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:38>
Comment (by aaugustin):
#19066 was a duplicate with a simple patch and tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:39>
Comment (by dyve):
I keep running into this issue. It's a really obnoxious test that certain
does not belong in ManyToManyField. It belongs in widgets if anywhere. But
why add this to every Many To Many? We don't show "Press Enter to submit"
help texts when displaying forms?
This functionality assumes a HTML select widget, and nowadays much more
different widget styles (scripted and otherwise) are available.
Basically, the text is a piece of hardcoded UI that should be completely
removed instead of toned down. The only reason not to remove this is
backward compatibility. If removing it is too big a step, it should be
moved to SelectMultiple widget.
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:40>
Comment (by gert):
1) As many people have stated (in various duplicate tickets) the help text
should not be in the database layer
2) Also, overriding the help text from the model definition has no effect
- so putting the help text in the database layer is breaking the model
definition
This has been going on for 4 years now, without resolution, please allow
me to propose a different approach:
Firstly it's a bug, there is no need to maintain any backwards
compatibility. Why don't we delete it from the database layer and then
opening a new ticket that states that the widget does not display the help
text. That way at least just the widget is broken, currently the widget
and the database layer is broken.
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:42>
Comment (by anonymous):
What's wrong with this patch?
https://code.djangoproject.com/attachment/ticket/19066/0001-Allow-
overriding-help_text-default-message-on-ManyTo.patch
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:43>
* stage: Design decision needed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:44>
Comment (by dyve):
Adding my support to absolutely removing this from the database layer. I
keep running into this. And sometimes it's just plain ridiculous (for
example when using `CheckboxSelectMultiple` for a ManyToMany.
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:45>
Comment (by anonymous):
This is the single most annoying bug on django, if dyve won't fix it I
will. For now I'll wait.
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:46>
Comment (by ramiro):
I've posted a proposal to the django-dev mailing list:
https://groups.google.com/forum/?hl=en&fromgroups#!topic/django-
developers/PqfHA3hvAxo
Feedback on is welcome.
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:47>
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"4ba1c2e785feecfa7a47aa5336a2b595f086a765"]:
{{{
#!CommitTicketReference repository=""
revision="4ba1c2e785feecfa7a47aa5336a2b595f086a765"
Fixed #9321 -- Deprecated hard-coding of help text in model
ManyToManyField 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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:48>
Comment (by Ramiro Morales <cramm0@…>):
In [changeset:"8c2fd050f80f528cc1609c1a7f16901194834831"]:
{{{
#!CommitTicketReference repository=""
revision="8c2fd050f80f528cc1609c1a7f16901194834831"
Made fix for #9321 less buggy and more effective.
Don't try to be smart about building a good-looking help string
because it evaluates translations too early, simply use the same old
strategy as before. Thanks Donald Stufft for the report.
Also, actually fix the case reported by the OP by special-casing
CheckboxSelectMultiple.
Added tests.
Refs #9321.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:49>
Comment (by Tim Graham <timograham@…>):
In [changeset:"e80de93af6a0a21a9063a55c4d6d20e3927243e9"]:
{{{
#!CommitTicketReference repository=""
revision="e80de93af6a0a21a9063a55c4d6d20e3927243e9"
Removed hard-coded help_text for ManyToManyFields that use a
SelectMultiple widget
Per deprecation timeline; refs #9321.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:50>
Comment (by Ramiro Morales <cramm0@…>):
In [changeset:"491419b5ffac3752a1c1804a763c017a2ed82e16"]:
{{{
#!CommitTicketReference repository=""
revision="491419b5ffac3752a1c1804a763c017a2ed82e16"
Made m2m fields form help_text munging specific to admin widgets.
Refs #9321 and follow-up to e80de93af6a0a21a9063a55c4d6d20e3927243e9.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:51>
Comment (by anonymous):
will this be implemented in 1.8? the ticket says 1.4 and the "press
ctrl..."-help text is still appended to any custom text.
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:52>
Comment (by timo):
Please look at the commits above for details. The Version field on the
ticket merely indicates what the current version was when the ticket was
reported.
--
Ticket URL: <https://code.djangoproject.com/ticket/9321#comment:53>