At the moment, drupal_clean_css_identifier is used for creating the classes for roles, which doesn't run through a lower case filter . If you have a role labelled with capitals such as "Content Editor", you can't use the generated classes (user-role-Content_Editor) because they fail JSON validation.

Changing this to drupal_html_class fixes this.

Simple patch attached which fixes this, let me know if anything else needs to be done with this.

CommentFileSizeAuthor
#4 json-errors-advanced-form.PNG16.07 KBdanielveza
#2 2874248-drupal-html-class-roles-2.patch586 bytesdanielveza

Comments

DanielVeza created an issue. See original summary.

danielveza’s picture

danielveza
he/him
Location Brisbane, AU
commented
StatusFileSize
new 2874248-drupal-html-class-roles-2.patch586 bytes
dman’s picture

dman commented

drupal_html_class() sounds good.
Would applying this cause regression for any sites currently using the current configs though? If so, I'd need to put in an update hook to fix everyones old configs.

What is the 'JSON' validation that's failing?

danielveza’s picture

danielveza
he/him
Location Brisbane, AU
commented
StatusFileSize
new json-errors-advanced-form.PNG16.07 KB

Attached an image of the JSON parsing fail for clarity. Any capitals in a class seems to make it fail.

I suspect it wouldn't need an update hook since you can't put any role classes with upper case characters into an advanced form rule at the moment and have it be parsed correctly.

dman’s picture

dman commented

looks like a helpful error message! Queer quoting. I wonder what's causing that.

If it's always been broken/unusable, then yeah, no update needed.

dman’s picture

dman commented

Was due to the syntax not being real json in the first place, and my handmade tokenizer getting it wrong

    // Enquote everything before trying to json it.
    $rule = preg_replace('/([a-z0-9_\#\-\. ]+)/', '"$1"', $rule);
dman’s picture

dman commented 
    $rule = preg_replace('/([a-zA-Z0-9_\#\-\. ]+)/', '"$1"', $rule);

may have been what was needed there,

Still, lower-casing everything as a policy also has merits.

  • dman committed 192fbf6 on 7.x-1.x 
    Issue #2874248 : Rule tokeniser was enquoting capitals wrong.
  • dman committed 48e6b01 on 7.x-1.x authored by DanielVeza
    Issue #2874248 by DanielVeza: Use drupal_html_class for roles
dman’s picture

dman commented
Status: Active » Fixed

Done both.
Repaired the tokeniser, and also flattening the class that's getting applied for roles.

Cheers!

  • dman committed e05d96f on 7.x-1.x 
    Issue #2874248 : Use drupal_html_class instead of...
dman’s picture

dman commented

TODO: one day switch to proper JSON in the configs there I guess.
Being almost-but-not-quite JSON is a silly thing. But JSON quoting is its own PITA to deal with in the UI, so I'd provided a relaxed interpreter there to be more succinct and easier for the admin. :-/

Status: Fixed » Closed (fixed)

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