WordPress.org

Making WordPress.org

Opened 15 months ago

Closed 6 months ago

#1903 closed defect (fixed)

Hardcoded strings in WordCamp.org plugin camptix-attendance

Reported by: kossmann Owned by: iandunn
Milestone: Priority: normal
Component: WordCamp Site & Plugins Keywords: has-patch
Cc:

Description

There are two hardcoded strings making it impossible to translate.

Attachments (4)

1903.diff (2.0 KB) - added by kossmann 15 months ago.
1903.2.diff (3.3 KB) - added by kossmann 15 months ago.
v2
1903.3.diff (3.3 KB) - added by kossmann 14 months ago.
v3
1903.4.diff (1.2 KB) - added by SergeyBiryukov 9 months ago.

Download all attachments as: .zip

Change History (16)

@kossmann
15 months ago

#1 @kossmann
15 months ago

  • Keywords has-patch added

#2 @iandunn
15 months ago

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

Hi Daniel, thanks for the patch!

This looks pretty good to me. The only minor things I'd change would be to use the wordcamporg text domain instead of camptix, and to always use the escaping translate functions (like esc_html__).

CampTix has it's own text domain because it's distributed in the WordPress.org repository, but the add-ons in the Meta repository should all use wordcamporg, so that they'll be included in our single GlotPress project.

Not everyone agrees with this, but personally I consider it a best practice to always use the escaping translate functions, to avoid any situations where malicious HTML is inserted into localized strings.

#3 @kossmann
15 months ago

Thanks for the feedback, here are the changes.

@kossmann
15 months ago

v2

#4 @kossmann
14 months ago

Hi, is everything OK with my changes? Should I do something else?

#5 @SergeyBiryukov
14 months ago

I think <strong>Note</strong>: Anyone with the secret link can access the attendance UI... should be a single string rather than two separate strings (Note: and Anyone with the secret link...).

Looks good to me otherwise.

@kossmann
14 months ago

v3

#7 @iandunn
14 months ago

  • Owner changed from kovshenin to iandunn

It looks like Note isn't a separate string in 3.diff, but I can make that change when I commit it. The rest looks good to me.

#8 @iandunn
14 months ago

Er, nevermind, I misread Sergey's comment. 3.diff is correct.

#9 @SergeyBiryukov
14 months ago

1903.3.diff makes the whole string bold, I didn't mean that :)

I just meant the string can be wrapped with esc_html_e() as is, no need to separate it. Occasional HTML tags in translatable strings are fine, unless thay wrap the whole string or placeholders (e.g. <code>%s</code>), in which case they can be moved out of the string.

Last edited 14 months ago by SergeyBiryukov (previous) (diff)

#10 @iandunn
9 months ago

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

In 4968:

CampTix Attendance: Internationalize strings

Fixes #1903
Props kossmann, SergeyBiryukov

#11 @SergeyBiryukov
9 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening to address comment:5 and comment:9 in 1903.4.diff.

Only the "Note" word should be bold, not the whole string. (Unless it was an intended change?)

#12 @coreymckrill
6 months ago

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

I think having the whole string bolded is ok. At this point if we change it back, the whole string will have to get translated again.

Note: See TracTickets for help on using tickets.