Opened 10 years ago
Closed 10 years ago
#5835 closed enhancement (fixed)
Allow a few more tags/attr on bp allowed tags filter.
Reported by: | hnla | Owned by: | boonebgorges |
---|---|---|---|
Milestone: | 2.2 | Priority: | low |
Severity: | minor | Version: | |
Component: | Core | Keywords: | has-patch |
Cc: | hnla |
Description
Running on from ticket ##3374
Perhaps allowing a few more elements and atts through the bp_allowed_activity_tags filter might be useful?
I would suggest adding <b>
, <i>
as two essential typographic visual conventions (not strong or em).
The anchor element would benefit from having the 'title' attr - this is arguably a more or as equally important attr as the provided 'rel' one.
The provided span tag in this context (similar in vein to the wp editor usage) is really only of use if one is able to also add the 'style' attr without this attr it's not really useful to general users who won't know what class to add nor developers/site admins to know what styles to apply to a possibly unknown token (granted probably the majority of users won't know what properties might be added via the style tag)
If there's agreement I can test and patch.
Attachments (1)
Change History (10)
#3
@
10 years ago
What are you suggesting regarding span?
That we add the style attribute, without it in this sort of context i.e outside external styles where only inline styles would be applicable, it's fairly useless, as a dev I won't necessarily know what class someone may decide to add or where the span appears thus can't target any specific styles in advance - but I'll concede that general users might be hard pressed to know how to use the style attribute, but think it might as well be allowed as not?
#4
@
10 years ago
- Milestone changed from Awaiting Review to 2.2
without it in this sort of context i.e outside external styles where only inline styles would be applicable, it's fairly useless,
I don't know about that. I've seen plenty of places where people use this kind of selector:
div.some-specific-class-name > span {
That suggests that a span alone could be useful in some cases.
I'm pretty sure the motivation for not allowing this attributes is, in part, that we don't see an obvious need to allow users to manually enter this kind of HTML into a status update. That said, *all* activity passes through these filters, not just status updates - so we should be more sensitive to the possibility that fully-formed markup may be provided in some cases.
On the other hand, we don't allow the 'style' attribute in any of our allowed_tags filters, and there are good reasons why we may want to leave this as the default. The activity stream should act like a "controlled" area. For example, in the case of blog posts, we strip inline images, and use the first image in the post (or the featured image) as a leader in the activity content. We do this to maintain uniformity in the appearance of the activity stream. Permitting BP components (or, heaven forbid, users) to send arbitrary styling into the activity stream seems like it's asking for trouble. Would you want an activity item's content to contain 64px pink Comic Sans letters?
So, in the case of 'span', I'm leaning toward leaving it as is. I'd welcome another dev's opinion, though.
hnla - Feel free to write up a patch for the other tags, and we can at least take care of that early in 2.2.
#6
@
10 years ago
Fair enough. it is a possible issue that could arise and for that reason best to avoid. However think 64px pink comic sans could work under the right circumstances, but I would mal-form the closing span to ensure all following elements inherited the same then it wouldn't look out of place. :)
I'll patch for the other tags/attr.
b
,i
,a:title
- yesWhat are you suggesting regarding
span
? That we remove it, or that we add thestyle
attribute?