[Django] #30134: Geodjango js template should use `|safe` for float values to avoid DECIMAL_SEPARATOR ruin the js syntax

12 views
Skip to first unread message

Django

unread,
Jan 25, 2019, 8:00:51 PM1/25/19
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
------------------------------------------+------------------------
Reporter: Sassan Haradji | Owner: nobody
Type: Bug | Status: new
Component: GIS | Version: 2.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------+------------------------
`contrib/gis/templates/gis/admin/openlayers.js` should use `|safe` on
float values to avoid DECIMAL_SEPARATOR (and probably other settings in
this category) ruin the js syntax by adding unexpected characters instead
of dot.

--
Ticket URL: <https://code.djangoproject.com/ticket/30134>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jan 25, 2019, 8:27:13 PM1/25/19
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+--------------------------------------

Reporter: Sassan Haradji | Owner: nobody
Type: Bug | Status: new
Component: GIS | Version: 2.1
Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------

Comment (by Sassan Haradji):

Fixed here: https://github.com/django/django/pull/10896

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:1>

Django

unread,
Jan 25, 2019, 8:27:38 PM1/25/19
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+--------------------------------------

Reporter: Sassan Haradji | Owner: nobody
Type: Bug | Status: new
Component: GIS | Version: 2.1
Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------

Comment (by Sassan Haradji):

Fixed here: https://github.com/django/django/pull/10896

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:2>

Django

unread,
Jan 25, 2019, 8:35:29 PM1/25/19
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+--------------------------------------

Reporter: Sassan Haradji | Owner: nobody
Type: Bug | Status: new
Component: GIS | Version: 2.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------
Changes (by Tim Graham):

* has_patch: 0 => 1
* needs_tests: 0 => 1


Comment:

It looks to me like `{% localize off %}` is meant to prevent formatting of
the value. Can you add a test that demonstrates how the issue happens?

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:3>

Django

unread,
Jan 25, 2019, 8:56:03 PM1/25/19
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+--------------------------------------

Reporter: Sassan Haradji | Owner: nobody
Type: Bug | Status: new
Component: GIS | Version: 2.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------

Comment (by Sassan Haradji):

`localize off` turns off `l10n` (look at `django/templatetags/l10n.py`) in
my case `l10n` is already turned off in my settings.

When `DECIMAL_SEPARATOR` is manually set in settings, `localize off` has
no effect on it, look at this comment copied from line 117 of
`django/utils/formats.py`:

```
# The requested format_type has not been cached yet. Try to find it in
any
# of the format_modules for the given lang if l10n is enabled. If it's
not
# there or if l10n is disabled, fall back to the project settings.
```
fall back to the project settings, means falling back to
`DECIMAL_SEPARATOR` (and other settings in its family). But just cause I
set a custom value for `DECIMAL_SEPARATOR` doesn't mean gis should break
on my django admin.

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:4>

Django

unread,
Jan 26, 2019, 4:31:09 AM1/26/19
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+--------------------------------------

Reporter: Sassan Haradji | Owner: nobody
Type: Bug | Status: new
Component: GIS | Version: 2.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------

Comment (by Claude Paroz):

I think that for your use case, it would be better to activate l10n for
your project and provide a custom format file
(https://docs.djangoproject.com/en/stable/topics/i18n/formatting
/#creating-custom-format-files). Is there anything preventing you applying
this suggestion?

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:5>

Django

unread,
Jan 26, 2019, 5:49:30 AM1/26/19
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+--------------------------------------

Reporter: Sassan Haradji | Owner: nobody
Type: Bug | Status: new
Component: GIS | Version: 2.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------

Comment (by Sassan Haradji):

I don't know if something is preventing me applying this suggestion or
not, I may be able to apply it, but it doesn't change the fact that
settings `DECIMAL_SEPERATOR` should not break js syntax in js templates in
no circumstances. Currently settings `DECIMAL_SEPARATOR` breaks above
mentioned js template regardless of the value of `L10N` (`False` or
`True`).

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:6>

Django

unread,
Jan 26, 2019, 10:12:04 AM1/26/19
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+--------------------------------------

Reporter: Sassan Haradji | Owner: nobody
Type: Bug | Status: new
Component: GIS | Version: 2.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------

Comment (by Claude Paroz):

This may be a documentation issue, we could warn that using the `l10n`
framework and maybe custom format files should be preferred over setting
the global `DECIMAL_SEPARATOR` which cannot be selectively deactivated
with `l10n` filters/tags.

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:7>

Django

unread,
Jan 26, 2019, 10:27:33 AM1/26/19
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+--------------------------------------

Reporter: Sassan Haradji | Owner: nobody
Type: Bug | Status: new
Component: GIS | Version: 2.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------

Comment (by Sassan Haradji):

If gis is not supposed to work with `DECIMAL_SEPARATOR` set in global
settings file, then this is indeed a documentation issue rather than a
bug. But I think there's room for improvement here, current behavior even
if documented is counter intuitive, because developer doesn't know
necessarily about internals of gis admin panel and doesn't expect setting
`DECIMAL_SEPARATOR` to break it. Documenting this would be appropriate if
it were some random php framework, but even if documented this behavior of
setting some settings breaks another completely unrelated thing is not
something I've ever seen in Django in my +9 years experience with it. I
think what would be acceptable is one of these options:
1. Remove/Deprecate things like `DECIMAL_SEPARATOR` in settings
completely, if it's not something fully supported lets not have it when
there are better alternatives that doesn't break things and do the same
job (format files.)
2. Support `DECIMAL_SEPARATOR` and similar settings "completely" which
means solving the issue described in this ticket and other possible
problems setting `DECIMAL_SEPARATOR` may cause.

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:8>

Django

unread,
Jan 26, 2019, 11:29:58 AM1/26/19
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+--------------------------------------

Reporter: Sassan Haradji | Owner: nobody
Type: Bug | Status: new
Component: GIS | Version: 2.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------

Comment (by Claude Paroz):

I would be tempted to go the `DECIMAL_SEPARATOR` deprecation route, but I
may completely miss some real use cases...

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:9>

Django

unread,
Feb 9, 2019, 7:20:09 PM2/9/19
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+------------------------------------

Reporter: Sassan Haradji | Owner: nobody
Type: Bug | Status: new
Component: GIS | Version: 2.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------
Changes (by Tim Graham):

* stage: Unreviewed => Accepted


Comment:

I guess the ticket should be accepted, even if the path toward resolution
is unclear. I think deprecating `USE_L10N` and the corresponding
formatting settings in favor of locale formatting files would simplify a
lot of things, but I'm not knowledgeable enough on the ramifications to
put together a proposal.

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:10>

Django

unread,
Jun 1, 2020, 4:49:59 AM6/1/20
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+----------------------------------------
Reporter: Sassan Haradji | Owner: Claude Paroz
Type: Bug | Status: assigned
Component: GIS | Version: 3.1

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+----------------------------------------
Changes (by felixxm):

* owner: nobody => Claude Paroz
* status: new => assigned
* version: 2.1 => 3.1
* needs_tests: 1 => 0


Comment:

[https://github.com/django/django/pull/12957 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:11>

Django

unread,
Jun 4, 2020, 5:00:32 AM6/4/20
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+----------------------------------------
Reporter: Sassan Haradji | Owner: Claude Paroz
Type: Bug | Status: assigned
Component: GIS | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+----------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"51250d2f123b694ab7e09c119cb72d4878266688" 51250d2f]:
{{{
#!CommitTicketReference repository=""
revision="51250d2f123b694ab7e09c119cb72d4878266688"
Refs #30134 -- Added test for {% localize off %} tag with format settings.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:12>

Django

unread,
Jun 4, 2020, 5:00:34 AM6/4/20
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+----------------------------------------
Reporter: Sassan Haradji | Owner: Claude Paroz
Type: Bug | Status: closed
Component: GIS | Version: 3.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+----------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"9e57b1efb5205bd94462e9de35254ec5ea6eb04e" 9e57b1e]:
{{{
#!CommitTicketReference repository=""
revision="9e57b1efb5205bd94462e9de35254ec5ea6eb04e"
Fixed #30134 -- Ensured unlocalized numbers are string representation in
templates.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:13>

Django

unread,
Jun 4, 2020, 5:01:23 AM6/4/20
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+----------------------------------------
Reporter: Sassan Haradji | Owner: Claude Paroz
Type: Bug | Status: closed
Component: GIS | Version: 3.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+----------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"810e656aca21a2389c1666e1cb132f6dd02a1e4b" 810e656a]:
{{{
#!CommitTicketReference repository=""
revision="810e656aca21a2389c1666e1cb132f6dd02a1e4b"
[3.1.x] Refs #30134 -- Added test for {% localize off %} tag with format
settings.

Backport of 51250d2f123b694ab7e09c119cb72d4878266688 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:14>

Django

unread,
Jun 4, 2020, 5:01:24 AM6/4/20
to django-...@googlegroups.com
#30134: Geodjango js template should use `|safe` for float values to avoid
DECIMAL_SEPARATOR ruin the js syntax
--------------------------------+----------------------------------------
Reporter: Sassan Haradji | Owner: Claude Paroz
Type: Bug | Status: closed
Component: GIS | Version: 3.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+----------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"acaa2015274fa79aa79184e1ee824cdc791f580e" acaa2015]:
{{{
#!CommitTicketReference repository=""
revision="acaa2015274fa79aa79184e1ee824cdc791f580e"
[3.1.x] Fixed #30134 -- Ensured unlocalized numbers are string
representation in templates.

Backport of 9e57b1efb5205bd94462e9de35254ec5ea6eb04e from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30134#comment:15>

Reply all
Reply to author
Forward
0 new messages