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

Allow all data attributes #3187

Merged
merged 4 commits into from
May 27, 2016
Merged

Conversation

nzgrover
Copy link
Contributor

@parndt New PR to allow all "data-" attributes.

#see https://github.com/flavorjones/loofah/blob/master/lib/loofah/html5/scrub.rb#L21
def scrub_attributes(node)
node.attribute_nodes.each do |attr_node|
next if attr_node.node_name =~ /\Adata-[\w-]+\z/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/\Adata-[\w-]+\z/ Why do we hardcoded this regex and don't let the dev to be able to override it in a config?

@nzgrover
Copy link
Contributor Author

@bricesanchez @parndt suggested he preferred this approach and asked me to submit a new pull request.

@parndt
Copy link
Member

parndt commented May 27, 2016

@nzgrover yes, I think what @bricesanchez is asking is whether we can move the regular expression itself to be configurable. I don't personally think we need to worry about that as it's easy enough for someone to write their own Presenter and Scrubber. Thanks for your work!

@parndt parndt merged commit c793da0 into refinery:master May 27, 2016
@sintro
Copy link

sintro commented Jul 12, 2016

Hmm, looks like "render_content_page" method from '/refinery/content_page' template still removes the data-attributes.
Here is the fragment of log warning after sanitizing:

<div class="image">
-        <img width="300" height="65" alt="Slide Background 2" title="title1" data-no-retina="true" src="/system/images/image.jpg" />
+        <img width="300" height="65" alt="Slide Background 2" title="title1" src="/system/image/image.jpg">
</div>

As one can see here, Scrubber has removed the 'data-no-retina' attruibute.

PS
I do have lines

def scrub_attributes(node)
  node.attribute_nodes.each do |attr_node|
    next if attr_node.node_name =~ /\Adata-[\w-]+\z/

    super
  end
end

in my section_presenter.rb

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

Successfully merging this pull request may close these issues.

None yet

4 participants