Revolution: Feature or Bug? Hashes in chunk names are not allowed

Created on 8 Mar 2017  路  22Comments  路  Source: modxcms/revolution

Summary

As far as I can see, hashes (#) are not allowed in chunk names. As long as a hash is inside a chunk name, it can't be saved. This is true for "element/chunk/create" as well as "element/chunk/update".

Why this would be cool

Meaningful chunk names with a selector like syntax:

body#home #banner.item

Can this break something?

I've renamed chunks in the database. Including one or more hashes in a chunk name doesn't seem to have any negative effect.

They show up in the manager, and can be used like normal chunks inother chunks and templates:

screenshot 2017-03-08 10 51 36

As far as I know, the only plugins using hashes are fastfield and pdoTools. I did a few tests and there don't seem to be any issues:

<h2>A normal chunk</h2>
[[$body_id_home_class_banner]]

<h2>A chunk with hash syntax:</h2>
[[$body#home.banner]]

<h2>Checking if fastfield works:</h2>
<h5>[[#1.pagetitle]]</h5>

<h2>Checking if you can mix fastfield and hashed chunks</h2>
[[#[[$#return1]].pagetitle]]

If there are any other issues, I would like to know about them.

Basically hashes should also be allowed in the names for snippet, plugins, tvs and templates. So far I haven't made up my mind whether this is useful or not, but who knows.

Step to reproduce

Create or update a chunk and add a hash to the name. It doesn't matter if the hash is at the beginning, inside or at the end.

Observed behavior

You can't save the resource as long it contains a hash

Expected behavior

Hashes should be allowed.

Environment

Any

All 22 comments

As you've already stated, you renamed this in the database, not via the MODX interface. Here hastags are stripped away. This leads be to belive it was never intended to be allowed. If the case had been that the manager allowed you to have a chunk with a hashtag, but it did not work, it would be another case.

Also, hashtags are used for pdoTool's fastfields, so it would render that extra useless if the parser was changed to accept hashtags.

Well, I did a few tests and fast field didn't mind about hashes.

They are expected to appear directly after the square brackets:

[[#123.pagetitle]]

Except for snippets that can come with or without an exclamation mark every elements starts with some special character.

So far, I couldn't find a single case where this caused problems with fast field.

The mystery about this is that we both couldn't find where and why hashes trigger the modx sanitation method or anything like that.

For all we know It might be some ancient stuff thats has always been there for no reason at all or something extremely clever that has protected us from unknown forces without us ever knowing.

I think it is worth investigating.

Just because it is there, doesn't have to mean it has to stay.

Like I said, you edited this directly in the database. If you do that you can break the site in a number of ways. Sanitation is not done when the data is fetched from the database, it is done BEFORE the data is stored.

I suggest closing this as a wontfix. Otherwise we might have to look at a million unicode problems too. This is simply beyond the scope of that the CMF has control over, when you directly corrupt the database via other tools.

I see no reason why # should not be allowed in element names. I think this is just a regex oversight.

"#" symbol is used at least in one widely-used Extra, FastField

As stated in the comments above, using # in element names does not break FastField.

Oh, indeed. Sorry for the noise

It's not noise. After thinking about it, it might trigger FastField unexpectedly though if used as the first character in an element name. Not sure how that works.

@opengeek I also tend to agree that this is caused by an overambitious regex rule. It doesn't break anything I could think of so far and I did a LOT of tests.

@OptimusCrime

I suggest closing this as a wontfix. Otherwise we might have to look at a million unicode problems too. This is simply beyond the scope of that the CMF has control over, when you directly corrupt the database via other tools.

I understand that you have more experience in that kind of problem, please explain why would we have unicode problems with # while we but don't have ones with other special characters like ".", "-", "_", etc.

That is not what I meant. At some point it was decided which characters were allowed in these names. We can go back and suggest a wide variety of new characters, but frankly I don't see the point. Why can't chunks be named with :, ;, , or either? I don't know. Should we support all of them? Perhaps?

it might trigger FastField unexpectedly though if used as the first character in an element name.
Not sure how that works.

Yes, it will be parsed as FastField tag and will produce an empty output.

What is the positive effect of using hash in element names? "Why not" - is not the reason.

Did you test this?

pdoparser.class.php, line 100 +

            $innerTag = ltrim($this->realname($innerTag), '!');
            $token = $innerTag[0];
            $innerTag = substr($innerTag, 1);
            switch ($token) {
                case '#':

Every chunk call should start with a $ ([[$test]] or possibly [[!$test]]), right?

So if I write [[$#test]] this should not trigger fastField ($innerTag[0] should be $, not #).

This is what I got from my tests.

What is the positive effect of using hash in element names?

It would be possible to use a css selector like syntax to name chunks.

Example: #banner

Much more intuitive...

"Why not" - is not the reason.

Why restrict something if there is no reason, isn't much better ;)

Every chunk call should start with a $ ([[$test]] or possibly [[!$test]]), right?

Yes, you are right.

It would be possible to use a css selector like syntax to name chunks.

And what?

Much more intuitive...

Why?!

[[$banner]]
[[$#banner]]
[[$_banner]]
[[$.banner]]

First variant is the best for me.

I guess this depends on everybody's personal view on things.

If I can look at the html output and guess the related chunk just by looking into its id and class.

Hashes (#) are completely banned inside chunk names:

Some examples:

  • body#banner ul li.first
  • #banner ul li.first

I agree that this might not be everybodies cup of tea, but that doesn't change the fact that I still can't see a solid reason why its not posible. That being said we have other things that need to be fixed, so no preasure here...

[[$body#banner ul li.first]]

Really?

In 2017, when we are trying to move all chunks in files and work in IDE with VCS support? Let the core team decide, but I`m against such "innovations".

I agree, for the sake of the matter, that using CSS selectors as the chunks seem redundant. What if you change a class, then you have to rename your chunk and all places where you include it. Not to mention the fact that with HTML5 you would generally try to avoid using IDs over classes. Secondly, if you write your CSS selector as body#banner ul li.first then you are doing something wrong. That is an unnecessary complex selector, it should just be something like #banner .first, which makes the approach to name the chunks after CSS selectors even more confusing.

Then again, like @bezumkin said, let this be up to the core team. I am not going to submit a PR for this as a disagree with the need for the functionality, but other should feel free to do as they see fit.

I'm mostly working with the same js/css libs, I have a habit to use the same snippets/chunks combos with various results.

I have to admit that my example was a bit over the top, on the other hand, everybody should be able use and abuse a system.

How about this approach?

banner_pdoRes_tpl
banner_pdoRes_tplWrapper

or this one

banner_mg_imageTpl
banner_mg_wrapperTpl

If I change the snippet used to output something I'd have to change the chunk names as well.

Naming the chunks after what they are actually doing (the result) seems to me much more sensible than naming them after the snippet (tool) they are used with.

In the end, this is not the topic of this thread. My question was why can't I use hashes and If there is not solid reason why they are forbidden, can we add them to the list of available characters.

Sofar I only learned that nobody really has an answer to that question..

To be clear, I don't mind if somebody closes this issue because there are (even remotely) possible conflicts. Better safe than sorry.

All systems have limitations. You can not write something else of letters, numbers and underscore symbols in variables in javascript and in PHP. Chunks, snippets, and plugins - it is a developer staff, they someday can be moved into the filesystem and used in common js/php files. They should follow rules that already used in the systems and well known for developers. So adding some staff because somebody wants or it just seems possible now does not make sense.
So I am closing as wontfix.

Don't get me wrong, I respect any decision that is based on facts, but this is another example for something that can't be change because of "reasons".

This might have been a bad or even a stupid idea. Yet, nobody shared his knowledge about possible issues or why there even was a rule in the first place.

"It has always been that way and therefore it must be right" doesn't bring anything forward.

Cheers,

Patrick

One good reason to not allow hash symbols in chunk names is:

The character "#" is unsafe and should
always be encoded because it is used in World Wide Web and in other
systems to delimit a URL from a fragment/anchor identifier that might
follow it.

_Source: RFC 1738_

If MODX would allow "unsafe" characters those strings have to be encoded all over the place to prevent possible issues that might arise when using "unsafe" characters.

Was this page helpful?
0 / 5 - 0 ratings