Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use data attribute to fetch admin.js options #2870

Merged

Conversation

k0kubun
Copy link
Contributor

@k0kubun k0kubun commented Apr 26, 2017

Background

Since Haml 4 doesn't escape interpolated Ruby script, the result of j I18n.t("admin.js").to_json is not HTML-escaped and it's working.

Problem

But in Haml 5, such interpolated script is HTML-escaped by default haml/haml#770.
Thus, with Haml 5.0.0.beta.2, it's broken like:

Uncaught SyntaxError: Unexpected token & in JSON at position 1
    at JSON.parse (<anonymous>)
    at admin:53

Changes

For security, it'd be good to have it in HTML-escaped form. So, I changed the template to fetch it from data attribute. Using data attribute, we can store JSON object in HTML-escaped form and fetch it safely.

@k0kubun k0kubun force-pushed the i18n-admin-js-data-attribute branch 6 times, most recently from d69faac to c6408ac Compare April 26, 2017 15:26
@k0kubun k0kubun force-pushed the i18n-admin-js-data-attribute branch from c6408ac to dc69ca1 Compare April 26, 2017 15:27
@mshibuya mshibuya merged commit 66f3dbd into railsadminteam:master Apr 27, 2017
@mshibuya
Copy link
Member

Thanks!

@k0kubun k0kubun deleted the i18n-admin-js-data-attribute branch April 27, 2017 04:24
ragesoss added a commit to WikiEducationFoundation/WikiEduDashboard that referenced this pull request May 26, 2017
The interpolated Ruby in a Haml :javascript filter now gets HTML escaped, breaking the way it was being used in the shared _head template to pass the set of valid languages and projects.

The fix is to pass it as an HTML dataset instead, and pull that into the Javascript afterwards. I followed the example of railsadminteam/rails_admin#2870
@ragesoss
Copy link

Thanks, this was a helpful example to follow for a similar problem in my app!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants