Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

classname should be escaped in template #903

Open
sunnysideup opened this issue Jul 1, 2019 · 8 comments
Open

classname should be escaped in template #903

sunnysideup opened this issue Jul 1, 2019 · 8 comments

Comments

@sunnysideup
Copy link

https://github.com/silverstripe/silverstripe-admin/blob/1/templates/SilverStripe/Admin/Includes/ModelAdmin_Content.ss#L19

Have not researched this in great depth, but when I do a search for $ClassName in *.ss files, this module pops up as using them a few times.

I dont think backslashes are allowed in css classes, so this may need to be changed?

@sunnysideup sunnysideup changed the title classname should be escaped? classname should be escaped in template Jul 1, 2019
@robbieaverill
Copy link
Contributor

robbieaverill commented Jul 1, 2019

I think changing this would break things for people. We've got implementations in supported modules that depend on the class name being a PHP FQCN in JavaScript and in CSS

@sunnysideup
Copy link
Author

change it in SS5?

@robbieaverill
Copy link
Contributor

Yeah that sounds better. To be honest, I don't think the PHP class name belongs in the frontend code at all... perhaps we can develop a better API for representing backend models in frontend templates and code...

@sunnysideup
Copy link
Author

sunnysideup commented Jul 1, 2019

We should add a new method to Text / Varchar called: HTMLClassName / SHortClassName (if there is not one already), that outputs the className like this: Silverstripe-Hello-World-Class in the way ModelAdmin uses it. Perhaps this method could even be added to DataObject (although I am sure ya'll are keen to reduce the size of this class).

In your templates you can then write $ClassName.ShortClassName or something like that. I am also thinking here about people adding the page type to the body tag in HTML (we often do this).

@michalkleiner
Copy link
Contributor

There already is a method to obtain it via ClassInfo. We currently use an extensions applied to Controller and DataObject having a simple method like

public function getClassShortName()
{
    return ClassInfo::shortName($this->owner);
}

@michalkleiner
Copy link
Contributor

I guess it can be made available via TemplateGlobalProvider on ViewableData.

@sunnysideup
Copy link
Author

@michalkleiner , we do that too, but I think a standard, universal "HTML ClassName" would ensure that more devs would use that standardised one, allowing modules to connect more seamlessly, etc...

@michalkleiner
Copy link
Contributor

Yep, consistent "core" way would be good, I agree with that.

Regardless of how we arrive there, the short name might not be what we need though, we used it to retrofit some features after SS3 upgrade, but it may, in rare cases, not always represent the class one wants to target in SS4.

So something like replacing \ with - or _ and lowercasing might be the way. Just thinking out loud.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants