Views row class filtering underscore to dashes on direct input

This is definitely a bug when you can't use grid system such as 960gs. There are classes like grid_4.

It looks like the following can be adjusted in /views/plugins/views_plugin_style.inc:

function get_row_class($row_index) {
    if ($this->uses_row_class()) {
      $class = $this->options['row_class'];
      if ($this->uses_fields() && $this->view->field) {
        $class = strip_tags($this->tokenize_value($class, $row_index));
      }

      $classes = explode(' ', $class);
      foreach ($classes as &$class) {
       //original - $class = drupal_clean_css_identifier($class);
       // new code on next line
       $class = drupal_clean_css_identifier($class, $filter = array(' ' => '-', '/' => '-', '[' => '-', ']' => ''));
      }
      return implode(' ', $classes);
    }
  }

drupal_clean_css_identifier by default changes underscores to dashes but this can be changed.

Here's a patch rolled against 7.x-3.x for the previous suggested fix, for testing. (Removed the faulty '$filters =' portion.)

I'm not sure this is a good idea. We intentionally use Drupal's default sanitization so that it follows standard Drupal behavior. This can be quite valuable most of the time. Plus, changing this midstream would change existing tokens and potentially break sites that are relying on existing classes that would change.

This is absolutely positively in no way a big. This is the designed behavior.

Happy to see it closed if it's not essential. Obviously Drupal themes should adapt to the way Drupal works, and not the other way round.

I guess it is a big deal for grids which are using underscores, but I'm not sure what to do about it. It's a big change midstream. :/

A foolish lack of consistency is the hobgoblin of open source.

Solution is really simple, search '_' and replace it with '-' in your grid css :D

Underscore is a valid character in CSS class names, so it shouldn't be filtered out.

Drupal 8 is adopting the SMACSS approach, and the new naming convention includes underscores in class names. Read more about it here: https://drupal.org/node/1887918

What @dcmouyard said. It messes up the (BEM) component syntax, since the view display should be the first component, not the row.

I agree that this is a bug that needs to be fixed.

Currently I'm working on a project with an external front-ender. CSS classes are his choice. It's my job to make it work, not to change his CSS. Drupal developers may understand the "just change your CSS" response, but I'd feel completely embarrassed explaining to a non-drupal dev "sorry, Drupal can't print an underscore there".

I cannot imagine that people actively rely on this feature (bug). However things may break because site builders made a typo and Views has been forgiving/autocorrecting until now. If we're worried about breaking stuff in the process of fixing a bug, I think it would be possible to add an update hook which:
1) scans existing views for row classes containing _
2) replace _ with - in those views
3) notify the person doing the upgrade which views have been changed
4) if module_exists('features'), remind the person doing the upgrade to re-generate the features containing those views

Seriously, we should rather fix drupal_clean_css_identifier() to accept underscores as well. Did anyone filed in issue against 7.x?

Re #12: You're right that the real problem is in drupal_clean_css_identifier(). The code comments in drupal_clean_css_identifier() seems to express what the function tries to do:

// By default, we filter using Drupal's coding standards.

The problem is, this small function tries to perform two tasks: 1) clean up invalid characters and 2) apply coding standards. We tend to prefer functions that "do one job and do it well", but I guess it's too late to refactor it into two functions in D7.

Fortunately, the function does allow us to bypass/influence the coding standards part using the second argument. If that is what we have, I think that is what we should work with right now.

Yeah this is indeed the problem. I really wonder whether we should maybe also try to fix the instances of core, but I guess they never have the problem, given that the css class cannot be configured.

@marcvangend
Your idea to update all the existing views seems quite nice. Do you want to write some update script for it, I won't have time, certainly.

Thanks. I'll see what I can do during this month's "community friday" at my company.

Views already has a helper function that avoids converting underscores, so here's an updated patch that uses that. However it does not address the backwards compatibility/upgrade issue.

See #2009584: Allow double underscores to pass through drupal_clean_css_identifier as per new CSS standards which is a related core issue (though focused on double underscores only at the moment). Obviously changing the core behavior would have similar backwards compatibility concerns as this issue does.

My suggestion there was to add a separate helper function to core to make it easier for contrib modules to do the bare-minimum filtering in situations like this one, but Views already has that helper function itself so it won't change much here :)

I really wonder whether we should maybe also try to fix the instances of core, but I guess they never have the problem, given that the css class cannot be configured.

Exactly, this is really only a bug in cases (like Views) where administrators are allowed to paste class names directly into the user interface; those should not be altered beyond what is absolutely 100% necessary.

IMO, this is a core issue not a Views issue. https://www.drupal.org/node/2009584#comment-9982145

IMHO this still is a Views issue, regardless of the double underscores issue in core.

I agree with John that the rules by which class names are cleaned in drupal_clean_css_identifier(), is a core issue. However Views needs to decide if it actually wants to apply drupal_clean_css_identifier() to user-entered class names. As said in #13, drupal_clean_css_identifier() aims to filter out invalid characters and enforce coding standards. Views should not care if users follow coding standards and only make sure that it produces valid markup; otherwise we end up with situations as described in #11. Therefore applying drupal_clean_css_identifier() is a fundamentally bad choice.

[edit]
Sorry, I didn't notice that drupal_clean_css_identifier() has a second $filter argument which allows you to override the character replacements. The default value for $filter converts underscores to dashes, but Views can choose to take that out and call the function like this:

drupal_clean_css_identifier($class_name, $filter = array(' ' => '-', '/' => '-', '[' => '-', ']' => ''));

So, using drupal_clean_css_identifier() does not have to be a bad choice if we tweak the $filter value.

I'd say this is BOTH a Core issue AND a Views issue. :-)

Given that Views already has a function in place to address this problem, but simply neglected to utilize said function in this instance, I do not think it's unreasonable to implement it now. An update function that fixes people's typos would be a nice touch, but I don't think it should make or break this bugfix. They are typos after all.

I applied the patch from #16 and it works for me.

This can be a real pain if you class things with the BEM syntax.

Agree with kevinquillen regarding BEM syntax compatibility (where the problem is exposed to me) and also patch in #16 works.

This is still an issue, and patch from #16 continues to work.

Marking RTBC based on this and the above positive feedback.

Turns out it applied to the last release, but not the dev branch.

Rerolled to apply to the dev branch. It had switched to using drupal_html_class(), but that calls drupal_clean_css_identifier() [ref] so I just changed it to views_clean_css_identifier() as the patch in #16 had.

Changed the corresponding test.

I'm tempted to move this back to RTBC since so many people reported #16 as working, and this is just a reroll of that.

But I guess since the function that was replaced changed, and there was a test that also needed to be updated, someone else should look over this to make sure nothing ridiculous is happening.

Adding related issue per #16.

And another related.

I can also confirm that #16 works well, thank you.

Since there hasn't been a new release of views in awhile, the patch in #27 applies to dev but not to the last release.

Here's a patch that applies to the last stable release, 3.13, for those who don't want to use the dev version. No interdiff or tests since this is just for that version.

To be clear, #27 still needs someone to review it against dev, in the hopes of getting it committed!

Here's another updated patch to work with views 7.x-3.14. No interdiff or test because this is just for the 3.14 release.

So to be clear, the patch against 7.x-3.x-dev to be reviewed is in #27 (it's a rerolled #16 and still applies to dev). If you have a moment to review it, please do so we can get this to RTBC!

Updated issue summary with pointers to patches.

And this is problem for views class(CSS class) in advanced settings too!!!

This tag doesn't add really much information.

Note: I'm not sure whether we should really touch views, when 7.51 will have a bugfix for that anyway (by enabling the setting).

Issue fixed in 3.16
Related issue: #2750021: View Styles Plugin - row CSS class name is forced to lowercase

(patch is the same)

This is not fixed in 3.16, and the patch in #2750021 doesn't address this.

It's easy to confirm it's not fixed:

https://simplytest.me install latest Drupal, Views, Ctools. Create a view. Give the view a class with underscores. Go to the view path. Inspect the view and see that the class's underscores have been replaced with hyphens.

Screenshots attached, if proof is necessary. (Edit: Sorry d.o resized the first one of these so annoyingly.)

Views backend:

Views inspected on the frontend:

So to be clear: the patch in #2750021: View Styles Plugin - row CSS class name is forced to lowercase replaces drupal_html_class() with drupal_clean_css_identifier(). The latter doesn't force-lowercase the string (the bug being addressed in that issue) but it does still convert underscores to hyphens.

The patches upthread (e.g. #33) instead replace drupal_html_class() with views_clean_css_identifier().

Here is an updated patch that applies to both 7.x-3.x and 7.x-3.16.

Looks like views_clean_css_identifier() now replaces underscores with hyphens too. So the patch in #41 is no good.

Patch from #41 is applied successfully with version 7.x-3.17. But it seems that it only retains CSS classes with underscores correctly from row classes but not when entered to Other -> CSS class which applies to the whole view.

Works great for me. Could we please bring this into Views? :)

Latest patch seems to work.

Drupal 7 introduced new setting to allow underscores in classes. You can opt in to this behavior with the following code in your settings.php file:

$conf['allow_css_double_underscores'] = TRUE;

For more information, see the change record at https://www.drupal.org/node/2810369

Setting the new variable does not seem to affect underscores in the Other: CSS class setting on the view itself as mentioned in comment #44.

The latest patch seems to be the best solution at this point.

I'd be happy to commit this if someone were willing to update it to take the "allow_css_double_underscores" variable into account.

Updated the description.

@Idebr and @DamienMcKenna, what is it you expect a new patch to achieve?

Should underscores be converted to hyphens after all if allow_css_double_underscores is not set or set to FALSE?

The current patch, the way I read it and not having tested it myself, seems to always preserve underscores in class names.

@DamienMcKenna, I'm not sure if this is what you had in mind, but I've updated the patch to preserve underscores in class names only when allow_css_double_underscores is set to TRUE. Also, rather than recreate the text filtering in core, views_clean_css_identifier() now just passes an updated filter array to drupal_clean_css_identifier().