When directly entering in a class name, not using a toking, that contains underscores into a row class, the underscores become filtered out and changed to dashes. This might be related to #1262630 where all input is run through a sanitize function in the row class. I would expect it to leave any entered input alone, but sanitize any token input. This is how the rest of Views works, I can enter in list classes and such and they are not run through a sanitize function.

Current status

The patch against 7.x-3.x-dev is in #27 applies and needs review.

If you want a patch against the most recent views release, 7.x-3.14, you'll find that in #33.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

altavis’s picture

Version: 7.x-3.0-rc3 » 7.x-3.0

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

akeemw’s picture

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.

thinkyhead’s picture

Status: Active » Needs review
FileSize
548 bytes

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

merlinofchaos’s picture

Status: Needs review » Closed (works as designed)

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.

thinkyhead’s picture

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.

merlinofchaos’s picture

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. :/

thinkyhead’s picture

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

thesame-’s picture

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

dcmouyard’s picture

Issue summary: View changes
Status: Closed (works as designed) » Needs work

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

amanire’s picture

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

marcvangend’s picture

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

dawehner’s picture

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

marcvangend’s picture

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.

dawehner’s picture

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.

marcvangend’s picture

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

David_Rothstein’s picture

Version: 7.x-3.0 » 7.x-3.x-dev
FileSize
493 bytes

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.

JohnAlbin’s picture

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

marcvangend’s picture

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.

jstoller’s picture

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.

kevinquillen’s picture

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

carsonblack’s picture

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

rootwork’s picture

Status: Needs work » Reviewed & tested by the community

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

Marking RTBC based on this and the above positive feedback.

The last submitted patch, 3: views_class_underscores-1371118-3.patch, failed testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 16: views-row-class-underscores-1371118-16.patch, failed testing.

rootwork’s picture

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.

Status: Needs review » Needs work

The last submitted patch, 25: views-row-class-underscores-1371118-25.patch, failed testing.

rootwork’s picture

Status: Needs work » Needs review
FileSize
1.65 KB
1.16 KB

Changed the corresponding test.

rootwork’s picture

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.

rootwork’s picture

rootwork’s picture

1mundus’s picture

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

rootwork’s picture

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!

rootwork’s picture

FileSize
1.65 KB

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!

rootwork’s picture

Issue summary: View changes

Updated issue summary with pointers to patches.

mibfire’s picture

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

prafull.addweb’s picture

Issue tags: +views
dawehner’s picture

Issue tags: -views

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).

Zekvyrin’s picture

Status: Needs review » Closed (duplicate)
Related issues: +#2750021: View Styles Plugin - row CSS class name is forced to lowercase

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

(patch is the same)

rootwork’s picture

Status: Closed (duplicate) » Active
FileSize
73.82 KB
111.5 KB

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 class with underscores

Views inspected on the frontend:
views class with hyphens

rootwork’s picture

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().

rootwork’s picture

Status: Active » Needs review
FileSize
1.29 KB

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

rootwork’s picture

Status: Needs review » Needs work

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

rsmylski’s picture

41 worked for me, it left my underscores.