Template notices html

[slaFFik]

We are an ordinary user, on Settings page.

To change the password we input incorrect current one, new and repeat it.
As the password is incorrect we get such an error: ​http://screencast.com/t/lZQtmvECC3t

Please look at the no space between sentences. If you look at the html source code - everything will be in 1 <p> tag.
If you look at the php source code around No changes were made to your account message, you will see that error messages are imploded using </p><p>. If you look at bp_core_render_message() you will see that there is no <p> tag around echo $content.

So we have html that is generated by BP:

<div id="message" class="bp-template-notice error">
    Your current password is invalid.</p><p>No changes were made to your account.
</div>

And the browser transforms it into:

<div id="message" class="bp-template-notice error">
    <p>Your current password is invalid.No changes were made to your account.</p>
</div>

So the styling is wrong.

Replicated on both BuddyPress 2.1.1 and 2.2 beta 1

[slaFFik]

See also #6030

[DJPaul]

Let's sneak a fix in for 2.2, as it ought to be pretty small.

[Mamaduka]

We already have wpautop for bp_core_render_message_content. Which should handle paragraphs. Issue is caused later in bp_core_render_message_content filters chain by wp_kses_data() which strips all HTML tags. We need to remove it or specify context.

[imath]

Hi,

I think the easiest is to avoid using html tags to join the feedback message and leave the filters the way they are. See 6112.02.patch.

@Mamaduka, i think we need to sanitize the output and removing the filter 'wp_kses_data' can be a trouble from my point of view.

[Mamaduka]

@imath

I don't like removing wp_kses_data as well, but it will strip all HTML. Do we have any internal kses wrapped with minimum allowed HTML?

[imath]

What is wrong with ​6112.02.patch ?

Is this the fact the feedback messages are only separated by a line feed and not "encapsulated" in a <p> tag ?

If so then, i prefer to have a specific kses function that will only allow the minimum tags. Best would be to filter 'wp_kses_allowed_html' and build some BuddyPress kses contexts (activity, messages, notices...)

But i think just using 6112.02.patch might be enough ? What do you think slaFFik and Mamaduka ?

[slaFFik]

@imath
You patch is the easiest, but I like @Mamaduka's more, as it will give ability to use html in template notices.
There is nothing that will be added in those notices by non-devs. And devs might want to have more control - even in using html there.

[slaFFik]

I'm talking about tooltips, for example.

[imath]

i don't know why but it seems very weird to me to print some html tags in cookies.. But at the very least, i really think it's safer we sanitize this output.

[r-a-y]

i don't know why but it seems very weird to me to print some html tags in cookies.

I kind of agree. Maybe we need to stop using cookies to save and display temporary messages. We probably should be using usermeta or PHP sessions.

---

I definitely do not think we should remove KSES as the patch in 6112-remove-kses.diff suggests. Especially for something that we're relying on a cookie for content.

imath has the right idea:

If so then, i prefer to have a specific kses function that will only allow the minimum tags. Best would be to filter 'wp_kses_allowed_html' and build some BuddyPress kses contexts (activity, messages, notices...)

---

However, for this specific problem on the Settings page, the existing implode() code is ugly, but imath's patch addresses the "no space" issue between multiple messages. I say we commit that for now and circle back to refactoring bp_core_render_message() to allow HTML for 2.3.

[Mamaduka]

Sorry for confusion, I only submitted patch to highlight where issue was caused.

I think it would be better, if we got with @imath's patch for this ticket and open new one for next major release.

r-a-y, I don't think using PHP session is best solution here. Since some managed WP host disable those (e.g. WPE). We definitely need to move whole impode/HTML part into render functions. Maybe do something like WP's *_setting_error() functions do.

[imath]

no problem Mamaduka, i'll commit the 6112.02.patch tonight and as jjj/r-a-y and you suggested, we could work on a ​BP_User_Feedback class to extend WP_Error in a near future

[imath]

In 9355:

Better separation of template notices on the user settings page

In case of more than one feedback messages are added to template notices of the user settings page, make sure the separation between each of them are clear enough.

Fixes #6112