Skip to:
Content

Opened 12 months ago

Closed 9 months ago

#7895 closed defect (bug) (fixed)

Gravatar Link printed as plain text

Reported by: vapvarun Owned by: imath
Milestone: 3.2.0 Priority: normal
Severity: normal Version: 3.0.0
Component: Templates Keywords: has-screenshots has-patch
Cc:

Description

At member's change cover image section

Gravatar.com link is printed as plain text due to use of esc_html_e

bp-templates/bp-nouveau/buddypress/members/single/profile/change-avatar.php

Line 18 & Line 77

Attachments (4)

active2.jpg (49.3 KB) - added by vapvarun 12 months ago.
Gravatar.com link
7895.patch (2.8 KB) - added by imath 12 months ago.
7895.2.patch (1.5 KB) - added by imath 11 months ago.
7895.3.patch (2.5 KB) - added by imath 9 months ago.

Download all attachments as: .zip

Change History (16)

@vapvarun
12 months ago

Gravatar.com link

#1 @DJPaul
12 months ago

  • Milestone changed from Awaiting Review to 3.2.0

#2 @imath
12 months ago

  • Owner set to imath
  • Status changed from new to assigned

Thanks for your feedback @vapvarun I’ll work on a patch asap.

@imath
12 months ago

#3 @imath
12 months ago

  • Component changed from Core to Templates
  • Keywords has-patch added
  • Version set to 3.0.0

7895.patch is my suggestion to fix the issue, another more straightforward way could be to use _e() instead of esc_html_e().

#4 @DJPaul
12 months ago

Current best practice is to HTML escape translations because translations could come from an untrustworthy source.

#5 @DJPaul
12 months ago

Or... just take the link to Gravatar out entirely.

#6 @imath
12 months ago

Well we have this situation for strings used into feedback messages anyway so the patch is simply creating a specific function to escape it out of the filters that are actually used. It’s the same issue than #7894 where it’s a bit difficult to remove the link there. Splitting strings is a bit risky imho but that’s another possibility.

#7 @DJPaul
12 months ago

For this specific issue, it's just the Gravatar link. #7894 is similar but different.

If there is no way to avoid having HTML in a translatable strings, then the easiest solution is to use sprintf and a translator's comment along the lines of "do not remove <a> tags". Adding in a template-pack specific sanitisation function for a string is over-engineering.

I think in #7894 we can try re-phrasing the sentence so that it is understandable and moves any HTML markup out of the translatable string (minus the placeholder). Over the next couple of weeks, I hope to have time to suggest some way of rephrasing the messages.

Last edited 11 months ago by DJPaul (previous) (diff)

@imath
11 months ago

#8 @imath
11 months ago

Ok, fine with me. I suggest 7895.2.patch then. Here's the result :

https://cldup.com/w3CbbTzFfE.png

Now people can use the Gravatar site of their area in the translation file, eg: https://fr.gravatar.com

#9 @imath
9 months ago

In 7895.3.patch I'm updating the patch I think @spdustin shared on the wrong ticket see the timeline. His patch improves mine by not forgetting to fix the same issue a few lines under when avatar uploads are disabled.

I think we should go with it.

@imath
9 months ago

#10 @spdustin
9 months ago

Whoops. My bad. I think I crossed patches.

Last edited 9 months ago by spdustin (previous) (diff)

#11 @imath
9 months ago

In 12216:

BP Nouveau: escape properly the user Profile Photo Gravatar fallback

Props vapvarun, DJPaul, spdustin

See #7895 (branch 3.0)

#12 @imath
9 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 12217:

BP Nouveau: escape properly the user Profile Photo Gravatar fallback

Props vapvarun, DJPaul, spdustin

Fixes #7895 (trunk)

Note: See TracTickets for help on using tickets.