My Web Site used to have a very simple filter that was not filtering class names. Now, many pages has different class names and there is too many pages to see them all one by one. I start to add rules to allow class names by pattern as I found new class but it because to be very annoying.

On my last attempt, I simply add multiple rules to allow every single class name but I'm afraid that will slow down the Web Site for nothing...
a*, b*, c*, ... y*, z*, A*, B*, C*, ... Y*, Z*
And even with all those rules, some class name are still striped...

According to W3C, there is a LOT more to consider than only a-z char on a class name:
http://www.w3.org/TR/CSS2/syndata.html#characters
[...] [class name] can contain only the characters [a-zA-Z0-9] and ISO 10646 characters U+00A1 and higher, plus the hyphen (-) and the underscore (_); they cannot start with a digit, or a hyphen followed by a digit. [...]
So, the class name 23 or -3r are not valid. However, the class name -r«3°C» IS valid (W3C rules, Firefox Browser and W3C Markup Validation Service accept it).

My request is to simply add a checkbox to allow the Bypass of the classname checking. I would be unchecked by default and has a description that explain Why it is so dangerous to bypass that check.

CommentFileSizeAuthor
#16 wysiwyg_filter-Bypass-rules-835202-16.D7.patch12.15 KBeugene.ilyin
#9 wysiwyg_filter-835202-9-Bypass-rules.patch11.98 KBAlan D.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gaellafond’s picture

gaellafond CreditAttribution: gaellafond commented

NOTE: I made a function a few months ago to validate class names. Fell free to use it if you want.

/**
 * Validate a CSS Class name according to W3C rules
 *   http://www.w3.org/TR/CSS2/syndata.html#characters
 *
 */
define('_VALID', TRUE);
define('_START_WITH_DIGIT', -1);
define('_CONTAINS_WHITESPACE', -2);
define('_CONTAINS_INVALID_CHAR', -3);

function _validate_classname($classname) {
  if (preg_match('/^[-]?[0-9]+/', $classname)) {
    // Do not start a class name with a digit or hyphen followed by a digit
    return _START_WITH_DIGIT;

  } elseif (preg_match('/[\s\x{00A0}]/', $classname)) {
    // No whitespace allow in a class name. It's treat separately because it's a common error.
    // NOTE: Regexp algorithm (\s) do not include the no-break space (U+00A0)
    // NOTE: Adding the no-break space itself instead of the Unicode code in the Regexp result in errors with some other characters...
    return _CONTAINS_WHITESPACE;

  } elseif (!preg_match('/^[a-zA-Z0-9\x{00A1}-\x{FFFF}_\-]+$/u', $classname)) {
    // It can contain only the characters [a-zA-Z0-9] and ISO 10646 characters U+00A1 and higher, plus the hyphen (-) and the underscore (_)
    // ISO 10646 characters U+00A1 and higher are define like this in regexp: /[\x{00A1}-\x{ffff}]/u
    return _CONTAINS_INVALID_CHAR;
  }
  return _VALID;
}
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented

Requesting, also.

Great module... but this feature is greatly needed.

Thank you,

rv0’s picture

rv0 CreditAttribution: rv0 commented

2nd the above. must-have feature.

psmx’s picture

psmx CreditAttribution: psmx commented

I agree. Would make things easier on my site also, since only administrators are allowed to use write HTML anyway. Was also thinking to create rules like (a*, b*, c*, ... y*, z*, A*, B*, C*, ... Y*, Z*).

Checkbox would be nice.

rajmataj’s picture

rajmataj CreditAttribution: rajmataj commented

Also agreed. Entering every letter of the alphabet followed by an asterisk is way too cumbersome. Giving themers and developers the opportunity to simply allow any class name would be extremely helpful. [markus_petrux], please comment on this as it's been a long time since it was posted. Thanks.

markus_petrux’s picture

markus_petrux CreditAttribution: markus_petrux commented

The reason to make this feature restrictive is that, at first, there should be no need to grant access to use CSS classes to end users, at least for a range of advanced editing scenarios.

Allowing a list of CSS class names is an advanced feature, meant to open just a few of them. Because if users can use almost any name, then they can use anything that's defined in site stylesheets, and that may break site layouts, or even allow users hide content, which might be a security issue.

If one needs to grant usage of so many CSS class names, I think alternatives to this module should be taken into consideration.

markus_petrux’s picture

markus_petrux CreditAttribution: markus_petrux commented
Status: Active » Closed (won't fix)
Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
Version: 6.x-1.5 » 7.x-1.x-dev
Status: Closed (won't fix) » Active

Allowing a list of CSS class names is an advanced feature, meant to open just a few of them. Because if users can use almost any name, then they can use anything that's defined in site stylesheets, and that may break site layouts, or even allow users hide content, which might be a security issue.

If this is the logic, then I can list about 15 other options within this filter that can be used to do this very easily.

One tiny step back: Filters range from [insert-date] to execute PHP, so why enforce a restrictive set of rules that do apply to a particular role, shall we say "content authors", to all roles that would benefit from this filter, including administrators that have more freedom?

We are talking about exposing this option to filter administrators. Filter admins effectively have, or can easily gain, access to any part of the system. OK, well some are easy (at least 3 filters expose PHP options), some ways are harder, but anyone with the know how can hack a site by disabling all filters and waiting for a logged in administrator to view the content created with a session hijack.

And in relation to other solutions. These are? The only one that works that I know of, is well, no filtering with a markup check.

All up, fantastic module that has an frustrating restrictive feature in relation to classes / id's.

Anyways, sorry for the rant, actually logged in to report another issues and was taken aback by the reasoning for the "feature". I am happy to roll a patch myself if you have changed your mind.

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
Status: Active » Needs work
FileSize
wysiwyg_filter-835202-9-Bypass-rules.patch11.98 KB

Very rough starter. Out of time playing with this for this project, maybe a more complete patch next time :(

I tried to do a global rule exclusion, but after getting classes to work, the ID and URL parts failed. I think that these are due to the primary regexp, meaning that id's may be requiring two regexp calls, but didn't investigate past that.

Test string was:

<div class="124" id="1234">test</div>
rooby’s picture

rooby CreditAttribution: rooby commented

I think this is definitely needed also.

There are lots of reasons you might want to use this module other than restricting classes.

aeski’s picture

aeski CreditAttribution: aeski commented

Yep, I think this module has a lot to offer with its filtering options, but not being able to select between whitelist/blacklist options for classes is a real pain.

Andrew_Mallis’s picture

Andrew_Mallis CreditAttribution: Andrew_Mallis commented

+1

In the context of a new build, it's feasible to structure a whitelist, but I just inherited a site with tons of historical data and I'd like to give all classes a green light for a particular input format, or else risk breaking things unexpectedly.

smndey’s picture

smndey CreditAttribution: smndey commented
Issue summary: View changes

It is a great module and it has much configuration flexibility, but all class selection is difficult. As per documentation under `valid_elements` we can have class for all elements something like this `@[class]`, but this haven't work for me I am using ckeditor not sure it might work for tinymce. All class support will fulfill the gap of the module.

JKingsnorth’s picture

JKingsnorth CreditAttribution: JKingsnorth commented
Title: Checkbox to allow all Class names » Checkbox to allow all class/ID names

+1 for this - could something like this also be implemented for IDs as well?

That would be useful for defining anchors to certain regions of text.

jglynn’s picture

jglynn CreditAttribution: jglynn commented

+1 for this too. Seems like a must-have to me. It's a pain if I want to add a class to some text to have to go in and define that class in Wysiwyg filter.

eugene.ilyin’s picture

eugene.ilyin CreditAttribution: eugene.ilyin as a volunteer and at DrupalJedi commented
FileSize
wysiwyg_filter-Bypass-rules-835202-16.D7.patch12.15 KB

Hi all

I've found error in previous patch. Variable $rule_key isn't defined in function _wysiwyg_filter_clear_messages.
I've investigated it and found that this function isn't need because it isn't used anywhere.
I prepared new patch without this function.

eugene.ilyin’s picture

eugene.ilyin CreditAttribution: eugene.ilyin as a volunteer and at DrupalJedi commented
Status: Needs work » Needs review

I think we can mark this issue as needs review.

IRuslan’s picture

IRuslan CreditAttribution: IRuslan as a volunteer and at DrupalJedi commented
Status: Needs review » Reviewed & tested by the community

Works great for me.

BartNijs’s picture

BartNijs CreditAttribution: BartNijs commented

Needed functionality, glad I found this patch. Works as expected. Thanks for the patch.

smndey’s picture

smndey CreditAttribution: smndey commented

It will be great if we can merge this patch in next release.

Ludo.R’s picture

Ludo.R
Location Brussels
CreditAttribution: Ludo.R commented

Patch #16 works well.

  • axel.rutz committed 11e098b on 7.x-1.x authored by eugene.ilyin 
    Code cleanup in issue #835202 by eugene.ilyin
  • axel.rutz committed 14a9844 on 7.x-1.x
    Issue #835202 by eugene.ilyin, Alan D.: Checkbox to allow all class/ID...
  • axel.rutz committed 50d6055 on 7.x-1.x
    Extracted whitespace changes from #835202

axel.rutz credited axel.rutz.

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin at MachbarMacher commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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