Ran into an issue where a css class with two dashes added to a panelizer page gets reformatted to a single dash.

CommentFileSizeAuthor
#9 double_dash_css_body-2287353-9.patch805 bytesmglaman
#9 2287353-interdiff.diff645 bytesmglaman
#8 panelizer_allow-double-dash-css_2287353-8.patch682 bytestom friedhof
#4 panelizer_allow-double-dash-css_2287353-4.patch691 bytestom friedhof
#3 panelizer-n2287353-3.patch717 bytesdamienmckenna
#1 panelizer_allow-double-dash-css_2287353-1.patch736 bytestom friedhof

Comments

tom friedhof’s picture

tom friedhof commented
Status: Active » Needs review
StatusFileSize
new panelizer_allow-double-dash-css_2287353-1.patch736 bytes

This isn't the most elegant fix for this, but solves the problem.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

I would if check_plain would be sufficient?

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Related issues: +#2287399: Allow ctools_cleanstring to retain multiple separator values
StatusFileSize
new panelizer-n2287353-3.patch717 bytes

No, check_plain() would not be sufficient. I'm also not sure I like this change either, I think instead we should work on patching CTools to not strip multiple dashes and leverage that API change, rather than removing any string security we have from the string replacement.

This patch leverages a new option I submitted as a patch for CTools: #2287399: Allow ctools_cleanstring to retain multiple separator values

tom friedhof’s picture

tom friedhof commented
StatusFileSize
new panelizer_allow-double-dash-css_2287353-4.patch691 bytes

Here is another patch that makes more sense, using a core function to sanitize the css class.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Maybe use drupal_html_class() instead?

Status: Needs review » Needs work

The last submitted patch, 4: panelizer_allow-double-dash-css_2287353-4.patch, failed testing.

tom friedhof’s picture

tom friedhof commented

I guess it couldn't hurt to use drupal_html_class() instead, to ensure the same string isn't sanitized multiple times.

tom friedhof’s picture

tom friedhof commented
StatusFileSize
new panelizer_allow-double-dash-css_2287353-8.patch682 bytes
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs work » Needs review
StatusFileSize
new 2287353-interdiff.diff645 bytes
new double_dash_css_body-2287353-9.patch805 bytes

Patch works great, fixes big issue when using BEM/SMACSS workflow. Updated patch to remove unneeded include to call ctools_include('cleanstring');

  • DamienMcKenna committed 5ca23be on 7.x-3.x 
    Issue #2287353 by tom friedhof, DamienMcKenna, mglaman: Use...
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Fixed

Committed. Thanks everyone!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

criz’s picture

criz
he/him
Location Vienna/Austria
commented

Unfortunately using drupal_html_class() doesn't work for double underscores (element).
See #2009584: Allow double underscores to pass through drupal_clean_css_identifier as per new CSS standards.