yiisoft / yii

Yii PHP Framework 1.1.x
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
4.85k stars 2.28k forks source link

Allow to change the wrapper around JavaScript code #2380

Closed teo1978 closed 8 years ago

teo1978 commented 11 years ago

The javascript code generated by CClientScript::registerScript() is always surrounded with /<![CDATA[/ and /]]>/

There doesn't seem to be an easy or clean way to change this behavior and have another wrapper instead, such as or no wrapper at all.

This is done by CHtml::script() (which can't be "overridden"; it could if CHtml was an application component rather than a helper class with static methods, for example). And this in turn is called from CClientScript::renderHead(). One can extend CClientScript and use the extended version as the "script" application component, but one would have to override the whole renderHead() method just to change one line in it, that's ugly.

A way should be provided to configure the wrappers for javascript code.

teo1978 commented 11 years ago

A more radical approach may solve this and other problems: not using helper classes full of static methods at all. That is, classes like CHtml shouldn't exist, because you can't override its functions and the framework uses them. However this would break backward compatibility in 1.x, and I guess you have already thought about this for 2.0.

cebe commented 11 years ago

@matteosistisette what is your usecase for this? Why do you want to change the wrapper?

teo1978 commented 11 years ago

For example: I find myself wanting to wrap javascript code in html comments, because it turns out GoogleBot is demential and crawls every javascript string that it thinks is an url, unless (they say, have to try if that's true) javascript code is hidden in html comments.

teo1978 commented 11 years ago

Also, if I'm not mistaken, in html5, with html syntax (as opposed to xhtml), the cdata wrappers are not needed, are they? I.e. you can put javascript code into script tags without cdata, can't you?

teo1978 commented 11 years ago

After looking a bit further into the HTML5 specs, it seems the CDATA wrappers are not only unnecessary, but INVALID in HTML5 with HTML syntax (as opposed to XHTML syntax).

So, if one has a HTML5 layout, the CDATA wrappers are producing INVALID HTML5 code, though probably it won't prevent any code from working.

http://www.w3.org/TR/html5/syntax.html#cdata-sections

CDATA sections can only be used in foreign content (MathML or SVG)

EDIT: on this specific part I am WRONG. it does pass validation.

samdark commented 11 years ago

Well, JavaScript is the same foreign content, isn't it?

cebe commented 11 years ago

There doesn't seem to be an easy or clean way to change this behavior and have another wrapper instead, such as or no wrapper at all.

  1. I can not find any spec saying using <!-- --> for javascript code. When you use these you will not be allowed to use for example i-- because -- is not allowed in a comment. also strictly spoken comment means to completely ignore everything in between which is normally not what you want for your JS. http://www.w3.org/TR/html5/syntax.html#comments
  2. no wrapper is even worse because you would not be allowed to do comarison < or > which will break your html.

CDATA is definitively the right way to go and there is no argument for changing it.

teo1978 commented 11 years ago

CDATA is something specific to XHTML. It's probably harmless but it's completely unnecessary in html5 with html syntax.

Your point 2 is plain wrong in HTML5 HTML syntax: scipt tag already treats everything inside it as a cdata block, Please have a look at the html5 specs. You do can use the lt ang gt signs without problems (on html5 compliant browsers of course)

No wrapper is perfectly fine in HTML5. The CDATA thing in HTML5 (non XHTML) will soon become obsolete BC-paranoid retro stuff.

Anyway, the choice should be left to the user. You are assuming that the output is XHTML.

And I can't see what harm an option for setting the wrappers could do btw.

teo1978 commented 11 years ago

@samdark no, the content of a script tag is not foreign content according to the specs

teo1978 commented 11 years ago

It's does validate, though

cebe commented 11 years ago

@matteosistisette you are right. The problem with your report is that you did not give any hint where to look in the specs to prove what you say ;) My first impression when reading this was that you have no idea what you are talking about, thats what lead to my answer and closing it. A simple link to the specs and a little explaintion would have made it much easier to see your point :) That for feedback. Now to the issue: I suggest to add another property like $closeSingleTags and $renderSpecialAttributesValue to CHtml that can be changed to achive HTML5 compilant output rendering.

teo1978 commented 11 years ago

The /<!--/ ... /-->/ validates too, even with an i-- inside it. From HTML5's perspective, it's not a comment, and it is as valid html code as the CDATA approach (both are hacks to work around bc issues, and one may be better than the other, depending on circumstances)

teo1978 commented 11 years ago

Sorry in the original report i didn't feel the need for linking to the specs, as the main issue for me has always been leaving the choice to the user.

samdark commented 11 years ago

btw., Yii2 uses HTML5 with CDATA as well.

teo1978 commented 11 years ago

I'm not saying it shouldn't be the default, just being able to change it.

cebe commented 11 years ago

btw., Yii2 uses HTML5 with CDATA as well.

it should not. Will try to find an easier source to read that up. The html specs descripe it from a parsers point of view :)

teo1978 commented 11 years ago

Btw please note I'm not an advocate of // wrappers themselves. They were used ages ago for reason that are now mostly obsolete. Now GoogleBot is demential in crawling strings that look like urls found in javascript code, but they (or some fan of theirs?) defend this behavior and suggest "protecting" javascript code from being scanned by putting it in html comments. I'm not sure whether standard-speaking that would mean the code shouldn't be interpreted even by the browser, as @cebe pointed out (certainly all current browsers do interpret it but if that was the case, they may start to ignore it in the future)..... and anyway it's not a thing one should be forced to do because of an arbitrary behavior of a bot that doesn't provide a robots directive to stop it.

But the problem I faced when I came across this situation is that I wanted to just give it a TRY (and see if I would stop getting ridiculous requests from googlebiot) and with current Yii I coudln't even try it. Whether to adopt a bad/questionable/unnecessary practice because a standard-and-commonsense-violating world-owning giant like google imposes it, or to keep with the standards at the cost of one's life, is a choice that should be left to the user, not taken by the framework :)

I think figuring out which is the best practice for wrapping js code is good for setting an appropriate default, BUT, from the framework's perspective, CClientScript is creating a Githubissues.

  • Githubissues is a development platform for aggregating issues.