escapejs breaks hyphen in filenames #1366

Closed
yakky opened this Issue Jul 29, 2012 · 4 comments

Projects

None yet

1 participant

Contributor
yakky commented Jul 29, 2012

This bug happens only in PlaceholderFields in custom models.
When saving a plugin in PlaceholderAdmin the plugin icon_src()/icon_alt() is filtered by django escapejs which converts the hyphen to its unicode value, breaking the icon URL (https://github.com/divio/django-cms/blob/develop/cms/admin/placeholderadmin.py#L228)

Example to reproduce:

  • Custom model using PlaceholderField
  • cmsplugin-filer-image
  • image with an hyphen in the filename

Steps

  • Insert a text plugin in the placeholder
  • Insert a cmsplugin-filer-image instance in the text area
  • select the image with the hypen in the filename
  • save
  • the image icon is broken

This is due the fact that cmsplugin-filer-image return as icon_src() the loaded image, but it's enough the below icon_src() definition in any plugin to show the bug:

def icon_src(self, instance):
    return settings.STATIC_URL + u"cms/images/plugins/image-hyphened.png"

A test to demonstrate this will follow soon

@yakky yakky added a commit to yakky/django-cms that referenced this issue Jul 29, 2012
@yakky yakky Testcase for #1366. Created a testcase and a crafted plugin to demons…
…trate the issue
0897f8a
@yakky yakky added a commit to yakky/django-cms that referenced this issue Jul 29, 2012
@yakky yakky Added testcase for page objects for #1366. This issue doesn't apply t…
…o page-defined placeholders as codepath differs
59d6353
Contributor
yakky commented Jul 29, 2012

Testcases added for both page-defined and custom model-defined placeholders. In the first case the issue is not present due to different codepath.
This issue occurs even when plugins are created directly into the placeholder; in this situation icon is not shown, so it's not apparent.

Awaiting further discussion before creating pull request with a fix.

Contributor
yakky commented Aug 18, 2012

@digi604 @ojii do you think you can push this in 2.3.1?

Removing escapejs from https://github.com/divio/django-cms/blob/develop/cms/admin/placeholderadmin.py#L228 should do no harm

Contributor
yakky commented Sep 5, 2012

PR Submitted to fix this

Contributor
yakky commented May 29, 2013

Ops, left open after merging

@yakky yakky closed this May 29, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment