Allow <body> attributes #6281
Allow <body> attributes #6281
Conversation
| * @return string | ||
| * @since 1.9.0 | ||
| */ | ||
| function elgg_set_body_attrs($body_attrs) { |
There was a problem hiding this comment.
Why not just use elgg_format_attributes with some additional params filtering?
There was a problem hiding this comment.
My bad, I overlooked elgg_format_attributes. As @mrclay prefers no filtering, then using it directly would work, which makes that function useless.
|
Tests are failing on coding standards violations, see: https://travis-ci.org/Elgg/Elgg/jobs/15621582 |
|
Not crazy about this implementation. Why don't we call the trigger in the view and use elgg_format_attributes. There's no need for a new function, nor to limit the attributes. data-* could be useful, e.g. |
| $body_attrs = array( | ||
| 'unauthorized' => 'attribute', | ||
| 'class' => 'may be multiple', | ||
| 'id' => 'i#s^-&&%\\/fr@ien!dl(y', |
There was a problem hiding this comment.
I'm not sure we want to support cleaning such mess. See what HTML 4.01 standard says about ID values
Only thing I've found in HTML 5.1 working draft is much more permissive. If that would be it, we actually shouldn't do such filtering, might think about escaping though.
So I would just say that developer CAN hurt himself and I'm not convinced we should prevent it in this case (BTW, it seems that you filter out : which is actually valid one).
|
Here, simplicity rules. |
|
I'm leaning towards merging it in current shape if @mrclay won't object |
Allow setting BODY attributes in page/default view
This allows plugin authors to add attributes to the body tag.
This patch comes after a request by @melvincarvalho to implement
<body about="this">as a feature to bootstrap Semantic Web and Linked Data support in Elgg.Usage:
elgg_register_plugin_hook_handler('output:before', 'page', 'my_body_attrs');Then: