User tests: Successful: Unsuccessful:
This PR adds addHTMLData
public function to JDocumentHTML
This can be used for creanting and adding values to data-* HTML5 attributes in any HTML element (by tag or by id).
This work in frontend and backend.
Examples of usage:
JFactory::getDocument()->addHTMLData('caption', 'img.caption');
JFactory::getDocument()->addHTMLData('caption', 'img.othercaption');
JFactory::getDocument()->addHTMLData('keepalive', array('uri' => '/index.php', 'seconds' => 60000));
JFactory::getDocument()->addHTMLData('somevar', 'somevalue', '#content');
caption
and keepalive
functions.None
Note: Improvements are welcome
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
IMO this too easily presents the ability to corrupt the rendered HTML and by its very design can cause major issues. Given this only supports altering an HTML element or a given element ID, one can easily call JFactory::getDocument()->addHTMLData('somevar', 'somevalue', 'div');
and would add their data-somevar
element to every div on the page.
Any changes like this to any library classes also should be accompanied with unit tests to validate behaviors.
I also don't really like that approach. Imho it's also perfectly fine to add inline JavaScript for such cases.
First of all, thanks all of you for the feedback. As you know i'm not and expert in Joomla as you guys are (still learning) so your comments are very important.
Proper camelCase is addHtmlData()
Yes true, that can be corrected.
But I'm not so sure that we need this if it's usage is to pass some attributes for JS.
We can do that in the component/module/plugin/library level (I think so)
That i know of, since there is no class to add/change html in the HTML body we can only do it with replaces in the rendered html or with overrides. As that is not possible in the core Joomla, don't think is possible.
IMO this too easily presents the ability to corrupt the rendered HTML and by its very design can cause major issues. Given this only supports altering an HTML element or a given element ID, one can easily call JFactory::getDocument()->addHTMLData('somevar', 'somevalue', 'div'); and would add their data-somevar element to every div on the page.
Yes you're right! Thanks.
I think this can be corrected by only allowing body
tag and id selectors.
Any changes like this to any library classes also should be accompanied with unit tests to validate behaviors.
That i have to see how to do.
I also don't really like that approach. Imho it's also perfectly fine to add inline JavaScript for such cases.
True, you can. It's working good as it is with inline js.
Ok. For what i read, you guys don't see the need for this PR.
I see the need and i think it adds more flexibility to pass data to javascript in joomla core instead of adding inline javascript that can present several problems when defering/async js.
For instance, the caption js (let's forget for now if can be replaced with HTML #7450). If you want to defer it's processing to the page onload event and make page load faster with that, how do you do it?
If you defer the caption.js, JCaption inline script will not work as it will return a javascript error.
As you know, this is will happen because JCaption is called in the head tag (page not loaded yet) and the caption.js, is this case, is only loaded on page onload event.
With values in data-* attributes you have no such problems because the is no inline js. All js in executed in the external js file, that can be executed in head or defered to page onload event.
By removing inline js, most of the scripts in behaviour.php could be defered to page onload event and make pages load faster.
This method can be used in several js files, which will make page load faster and better user experience, and with that better SEO.
But, of course, maybe there are cleaner ways to do this than this PR.
One idea that occurs to me is using data-* attributes directly in the external script tag, for instance:
<script src="/media/system/js/caption.js" data-caption="img.caption" async="true" /></script>
W3C HTML5 Recommendation says data-* are global attributes so data-* can be used in any tag, including the script tag:
Custom data attributes (e.g. data-foldername or data-msgid) can be specified on any HTML element, to store custom data specific to the page.
Source: http://www.w3.org/TR/html5/dom.html#global-attributes
So i will close this PR and investigate if this solution can be applied in a simplier and cleaner way and with B/C.
thansk agian for your feedback
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-11-24 21:57:18 |
Closed_By | ⇒ | andrepereiradasilva |
I totally support data attributes, don't get me wrong. It is THE PROPER way to pass vars from PHP to JS. But that can be done already thru Jlayout, or Layouts, so instead of having addScriptDeclaration('whatever');
you echo those params/vars into the actual html part wrapped in a data-attr.
The way that could work is demonstrated in #8278
Without building all HTML output in something like DOMDocument
, you don't without complex regex's.
Or you can you echo a simple span element
echo '<span data-url="' . $url . '" data-time="' . $refresh_time . '"></span>';
this will be ok for keepalive() in behaviour.php
@dgt41
I didn't even thought to try that
But two things to consider with that method:
But yes, it's and alternative that works as it is.
I think this can be corrected by only allowing body tag and id selectors.
Bad idea. If the spec allows data- attributes anywhere, any API Joomla implements shouldn't restrict this. So for an API endpoint to be useful, it would almost need to accept DOM style selectors and the method would have to be smart enough to decipher that and modify the right elements. So to do it "right", it immediately becomes far too complex to maintain at a high level IMO.
the alternative is JHtml::_('script',...) allows to add data-* values to the script tag and then the external js script can use it's own data attributes
Adding support for an attribs array to JHtml::script()
, JDocument::addScript()
, and the rendering support needed in JDocumentRendererHead::fetchHead()
is rather trivial. Actually, it's copying the existing mechanisms for JHtml::stylesheet()
and the associated methods. Just typehint the parameter as an array if someone does it. There are places passing a boolean false in for that attribs array for the stylesheet API and since Joomla does absolutely zero validation on this, it's going to become an issue when https://github.com/joomla/joomla-cms/blob/3.5.0-beta/libraries/joomla/document/html/renderer/head.php#L118 is changed to call the namespaced class Framework class which typehints to an array.
Proper camelCase is
addHtmlData()
But I'm not so sure that we need this if it's usage is to pass some attributes for JS.
We can do that in the component/module/plugin/library level (I think so)