Skip to:
Content

BuddyPress.org

Opened 11 years ago

Closed 9 years ago

Last modified 8 years ago

#4857 closed enhancement (fixed)

Automatic download of translations from translate.wordpress.org

Reported by: djpaul's profile DJPaul Owned by: djpaul's profile DJPaul
Milestone: 2.0 Priority: normal
Severity: normal Version:
Component: I18N Keywords: has-patch
Cc:

Description

We should automatically download a translation for BuddyPress, and keep it up to date, if the user has a locale set. Attached file is a working proof-of-concept that you can drop into mu-plugins.

This is important to do because we make it very hard at the moment for people to discover that translations are available; glotpress' UI is not meant for end users, and we don't do a great job of exposing the rosetta sites on bporg.

Attachments (9)

translatecode.php (4.0 KB) - added by DJPaul 11 years ago.
language-packs-002.patch (1.2 KB) - added by DJPaul 11 years ago.
translatecode-002.php (4.0 KB) - added by DJPaul 11 years ago.
translatecode-003.php (3.9 KB) - added by DJPaul 11 years ago.
4857.01.patch (15.6 KB) - added by DJPaul 11 years ago.
4857.02.patch (14.8 KB) - added by boonebgorges 11 years ago.
4857.03.patch (15.1 KB) - added by DJPaul 11 years ago.
4857.04.patch (15.0 KB) - added by DJPaul 11 years ago.
4857.05.patch (1.5 KB) - added by boonebgorges 11 years ago.

Download all attachments as: .zip

Change History (44)

@DJPaul
11 years ago

#1 @DJPaul
11 years ago

My only naggle with this is the get_glotpress_locale method. It'd be good if we can get an endpoint added to translate.wordpress.org so that we could give it the WordPress locale code, and get the glotpress locale code back.

#2 @boonebgorges
11 years ago

Pretty slick, DJPaul. This'd be great functionality to bake into BP. A couple questions:

  • In addition to the weekly check, maybe run during our upgrade routine?
  • There may be setups where WP_LANG_DIR is not writable. It looks like wp_mkdir_p() will fail gracefully in these cases. But do we want to throw an alert so that the admin knows he'll have to fetch the translations himself?
  • Do we have any idea how prevalent it is for people to modify the default language packs? If people have customized them without changing the locale/filename, their stuff will get overwritten.
  • Following up on these last two points, how would you feel about prompting the admin instead of doing it in the background? Or: A prompt by default, but a checkbox "Do this automatically in the future". I feel a bit uneasy about changing files on the user's system without some sort of action on his part first.

#3 @DJPaul
11 years ago

New patches.

translatecode-002 changes:

  • Use WEEK_IN_SECONDS constant
  • Correct WP_LANG_DIR path from WP_LANG_DIR/plugins/buddypress/{domain}-{locale}.mo to WP_LANG_DIR/plugins/{domain}-{locale}.mo -- see [WP22346]

language-packs-002 is a change to bp_core_load_buddypress_textdomain() to look in a new location. This fits the upcoming WP language pack convention and prevents overwriting of existing translation file.

  • We could run this on install as long as we can trigger it in a non-blocking way; maybe fire a one-time-only cron.
  • If we throw an alert if WP_LANG_DIR is not writable, we should somehow do it only after finding that a language pack exists for the current locale ("Hey, we tried to help you, but...").
  • I think auto-updating the language file is fine, and cool. Eventually, this is going to happen when people install a new plugin from WordPress.org.

#4 @boonebgorges
11 years ago

I think auto-updating the language file is fine, and cool. Eventually, this is going to happen when people install a new plugin from WordPress.org.

According to whom? It seems awfully intrusive to me. I can't think of anyplace in WordPress where the filesystem is modified without explicit action by the user. This goes especially for a case where a WP install is phoning an external service (albeit a relatively benign one like wordpress.org) and automatically propagating changes based on that service. Even services like iOS, etc ask for you to confirm updates.

In addition, automatic updates would wreak a certain sort of havoc with people who keep their codebases under version control and use deployment tools.

#5 @DJPaul
11 years ago

I'm not convinced the comparison to iOS holds up here; Chrome autoupdates just fine.

The main ticket for language packs seem to be #WP18200, and there's some discussion if you read through the comments. I remember talking with people about this (and auto-updates in WordPress in general) around WordCamps for a couple of years.

In addition, automatic updates would wreak a certain sort of havoc with people who keep their codebases under version control

And those people would turn it off. I see this as an example of the build-for-80%-of-the-users thing (I'm willing to best most WP sites aren't kept in version control).

#6 @boonebgorges
11 years ago

If I'm understanding the latest patch for that WP ticket correctly, the language pack updates are not happening in the background, they're being triggered at the end of a separate (plugin/theme/WP) upgrade process, which presumably was auto-triggered.

I'm not a fan of invisible autoupdate like Chrome, but whether or not that's the wave of the future, the fact is that this case is much more like iOS than like Chrome. Chrome is developed, deployed, and controlled by a single entity. The iOS App Store is full of contributions from many developers, far more akin to WP plugins, themes, translations. If it does turn out that in-the-background updates are instituted for WP more generally, then at least the user would not be surprised by having BP update language packs automatically. As it stands, this'd be the only part in a large system that does something behind the user's back. I find that pretty uncomfortable. A one-time prompt would be enough, IMO.

#7 @DJPaul
11 years ago

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

translatecode-003.php​ takes advantage of some new data that markoheijnen very kindly added to one of the APIs in GlotPress, which is now live on translate.wordpress.org.

#8 @boonebgorges
11 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I don't think you meant to close :)

#9 @DJPaul
11 years ago

A blocker for this is http://glotpress.trac.wordpress.org/ticket/236 -- going to start by trying to create a patch for it, and then see about getting it accepted/merged/deployed.

#10 @DJPaul
11 years ago

  • Owner set to DJPaul
  • Status changed from reopened to assigned
  • Type changed from enhancement to task

@DJPaul
11 years ago

#11 @DJPaul
11 years ago

4857.01 implements this feature. Feedback and/or testing appreciated:

  • A cron runs once weekly. If an updated translation exists on translate.wordpress.org, an option is set.
  • When the option is set, the update badge in the wp-admin dashboard is set.
  • On wp-admin/update-core.php, a new section is added for "Update BuddyPress Translation" which does what it says.
  • When the language is updated, the timestamp is stored in another option. This option is used in the future cron checks in a IF-MODIFIED-SINCE header, which GlotPress now supports (#GP236), to avoid wasting processing time on GlotPress and client download time.
  • I added a new location for load_textdomain inside the upload folder; a) to be sure we can write to the folder on most sites, b) so any site's existing translation is not overridden by the automatically downloaded one.

The current patch hardcodes the checked translation to Italian, to make testing easier. There's a method where this is set, so easy to change if you want.

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

#12 @boonebgorges
11 years ago

This is looking really great, DJPaul. I think it's going to be a model for the WP implementation.

I only ran into one issue while testing. In buddypress_translate() and bp_admin_maybe_bump_update_count(), you bail in the following conditions:

if ( ! bp_is_network_activated() && is_network_admin() )
//...
if ( ! is_admin() || bp_is_network_activated() || ! bp_is_root_blog() )

(a) AFAIK, on multisite, all plugin/theme updates happen on the network admin. So, the first instance prevents the update from ever taking place on an installation where BP is activated only on a single site.
(b) The bp_is_network_activated() check in the second condition means that it'll never be possible to see/do an update when BP *is* network activated.

So, it looks like updates are totally blocked in every case where BP is running on Multisite.

It seems to me that we can probably just drop both of these checks, though please let me know if I'm missing a problematic case. See 4857.02.patch.

(Small note: in 4857.02.patch, I also un-aligned the equals-sign alignment in bp-loader.php. Bumpbot is going to change the the version and db_version values, which causes your patch not to apply. If you care about this alignment, maybe wait until committing to apply it.)

#13 @DJPaul
11 years ago

Thanks for reviewing. Essentially, the checks were an attempt to ensure we only modify the wp-admin and run the update checks, only if:

  • We are on the BP_ROOT_BLOG

For multisite when our menus are in the network admin, we need to run in the content of the BP_ROOT_BLOG.

#14 @boonebgorges
11 years ago

I see. I think the BP_ROOT_BLOG stuff makes sense as a general rule for BP panels. But this is a special case: we're modifying the Updates panel, which only appears in the Network Admin no matter what the BP_ROOT_BLOG is, and no matter how BP is activated (network or no). So maybe the most effective check would be if ( ! is_network_admin() ) return;

@DJPaul
11 years ago

#15 @DJPaul
11 years ago

4857.03.patch removes the testing code from the patch, fixes a problem where .po were being downloaded instead of .mo ;), and reorganises the function order to make the class more readable. Tested and verified correct behaviour on regular and multisite.

Going to commit this next week unless any further feedback.

@DJPaul
11 years ago

#16 @DJPaul
11 years ago

4857.04.patch includes the change from [BB4934] ("Allow partial global/local .mo file loading") because it's a good enhancement and I'm modify the function anyway. I can commit this separately if preferred.

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

#17 @boonebgorges
11 years ago

Changes look great to me, DJPaul.

#18 @djpaul
11 years ago

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

In 7097:

Introduces automatic download of BuddyPress translations from translate.wordpress.org. Fixes #4857

BuddyPress now uses cron to periodically check if the version of the current locale's translation on translate.wordpress.org is more recent than the installed version.
If an update is available, the "Updates" badge's count in the WordPress admin menu is incremented, and a new button on the Updates screen allows a site admin to update the translation to that latest version.
This improves user experience for everyone who wants to use BuddyPress in their native language.

Thanks to markoheijnen for help making changes to GlotPress to enable this functionality.

#19 @johnjamesjacoby
11 years ago

  • Keywords needs-testing removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

This is a great patch and an amazing effort, but I reverted r7097 for a few reasons that I'll enumerate below:

  • Scaling: The .org GlotPress installation isn't built with the influx of traffic from pings and bulk file downloads in mind. We run the risk of accidentally bringing down the house for a relatively small amount of our current audience.
  • Security: In short, strings in BuddyPress that are not escaped (using esc_attr__()|esc_attr_e()|esc_html__()|esc_html_e()) become vulnerable to XSS via their connection to an externally linked library that is outside of our immediate control.
  • Timing: Automatic translation downloads are coming to the entire plugins directory as soon as June/July, about the time 1.8 is due to be released.
  • Code: The languages should be in wp-content/languages, and not uploads. Looks like we're requesting updated zips when it's not necessary to quite yet.
  • Setting an Example: Having our translations in GlotPress is a luxury that other plugins don't have. I'd hate for us to be the catalyst for other plugins going the route of installing their own GlotPress instance, and trying to solve these problems when we're so close to being there for everyone hosting in the plugins directory.

I've asked Nacin to chime in more about the future plans, since he's been working on this a bit already, and will be leading the effort in June/July. I've asked him to give some feedback about the approach also, since he's already done some prototyping.

In short, I'm happy this was worked on, and excited to hear more from Nacin about how we won't need to maintain our own code to handle all of this. :)

P.S. - Sorry for not chiming in to this ticket sooner, before the commit tonight. I wasn't keeping an eye on it specifically, and wasn't aware the feature was so close to feature complete.

Last edited 11 years ago by johnjamesjacoby (previous) (diff)

#20 follow-up: @nacin
11 years ago

Okay, so, this looks awesome. Fantastic job, Paul. Unfortunately, I agree with the revert for now. JJJ summed that up really well. Let me go into further detail. Here's three major reasons why:

  • Pulling directly from translate.wordpress.org is a scaling issue. Simply put, GlotPress does not scale well (in general). Worse, these files are generated on the fly and served through PHP, which actually has the potential to overload our load balancers once this is in the wild. We had some issues with our load balancers caused by mass plugin updates just last month.
  • There are some major security concerns here. Basically, nearly every string in BuddyPress is now vulnerable to XSS, which can be exploited by a user getting a string approved on WordPress.org and suddenly pushed to all installs everywhere. Very bad. This is the number one reason why core does not do this yet, despite there having been a few patches for it.
  • See JJJ's point re: setting an example. As a sister project, BuddyPress gets (well-deserved) special treatment, but it also comes with additional responsibility. Likewise, while another plugin could implement this with their own GlotPress install, security concerns are much more sensitive because WordPress.org is the vector.

So here's the good news! As JJJ said, this is coming to WordPress.org for all plugins by the end of June, in the form of a plugin (currently in development). That plugin will be included in core in 3.7 (due out later this summer). And we're working to create a system (and ecosystem) that handles all of these issues, including scaling and security. And, plugins will not need a single new line of code to make it work.

While I'm here, let me provide some quick feedback on the commit anyway:

  • This results in users seeing a new update every time a string is approved in their language. There needs to be some throttling to the user experience. We are planning something a bit different for core.
  • This should actually be stored in the languages directory, not in uploads. And actually, BuddyPress needs to be using load_plugin_textdomain() here (not just because it is the right function, but because that's what the core implementation will hook into). Can that actually be changed for BP 1.8?
  • This won't work for networks that have different locales on different sites, which is fairly common when translations are involved. If multiple locales are used on the same site (such as per-user, which is also not uncommon on community sites), it's possible for the cron to actually stomp the options. We have ideas for this too.
  • The 'update check' itself also triggers a file download, even though it is discarded. This will have an effect on scaling WP.org, one, but it will also result in a slower cron, actually slowing down people's servers. This could even cause hosts to block traffic to translate.wordpress.org, which would suck especially since it'd block our translate-all-plugins effort.
  • .po should probably be downloaded as well (something we are looking into), as by downloading just a binary file, we actually increase security concerns (no way to sanity check what we just installed blindly on a server, and it might also scare hosts). MO files also do not have headers, which means we can't read headers from them (which we can with .po). Good when you are building something

I should have a prototype in the next few weeks. It'll be backed by a new WP.org API, the code for which will most likely be public. I'll make sure I get you guys involved very early. This is really great, and the timeline for BP 1.8 final is a full month later than the time I want a beta out.

#21 follow-up: @boonebgorges
11 years ago

I can't speak to the scaling issues of GlotPress and the wordpress.org infrastructure, but they sound like serious issues. At the same time, I can't help but feel disappointed and frustrated that these points weren't raised earlier. DJPaul's initial patch for this ticket, which demonstrated the basic method, was posted here months ago, and progress on the ticket has been announced publicly many times in dev chat and on bpdevel.

==

Responding to nacin re load_plugin_textdomain(). I think the reason we haven't used it in the past is because we don't ship our translations along with BP, and we actively encourage people to store them in wp-content/languages. load_plugin_textdomain() with its deprecated $abs_rel_path param, does not make it obvious how to support this setup. I could only make it work by walking up the directory tree in $plugin_rel_path. See 4857.05.patch. Am I right that this is the only way to make load_plugin_textdomain() load .mo files from wp-content/languages?

#22 in reply to: ↑ 21 @johnjamesjacoby
11 years ago

Replying to boonebgorges:

I can't speak to the scaling issues of GlotPress and the wordpress.org infrastructure, but they sound like serious issues. At the same time, I can't help but feel disappointed and frustrated that these points weren't raised earlier. DJPaul's initial patch for this ticket, which demonstrated the basic method, was posted here months ago, and progress on the ticket has been announced publicly many times in dev chat and on bpdevel.

I understand and share your sentiment. We've improved our peer review each release, and this one was missed until the very last minute.

Responding to nacin re load_plugin_textdomain(). I think the reason we haven't used it in the past is because we don't ship our translations along with BP, and we actively encourage people to store them in wp-content/languages. load_plugin_textdomain() with its deprecated $abs_rel_path param, does not make it obvious how to support this setup. I could only make it work by walking up the directory tree in $plugin_rel_path. See 4857.05.patch. Am I right that this is the only way to make load_plugin_textdomain() load .mo files from wp-content/languages?

You're correct about this being the reason why, in addition to us wanting to control the ability to both have an external mo file in wp-content/languages, and eventually bundle them in core (per this ticket.)

Nacin: Would calling the 'plugin_locale' filter in our own function suffice? If not, it looks like load_plugin_textdomain() would need a patch to support using the /wp-content/languages/ path, either to un-deprecate the $abs_rel_path argument, or add a 4th parameter to assume that path by default.

#23 in reply to: ↑ 20 @SergeyBiryukov
11 years ago

Replying to nacin:

MO files also do not have headers, which means we can't read headers from them (which we can with .po).

Actually, they have.

I've exported translations from GlotPress as an .mo file and used this snippet to display the headers:

require( 'wp-load.php' );

$mo = new MO();
$mofile = WP_LANG_DIR . '/ru_RU.mo';
if ( ! $mo->import_from_file( $mofile ) )
	return false;

print_r( $mo->headers );

Output:

Array
(
    [PO-Revision-Date] => 2013-05-24 03:08:52+0000
    [MIME-Version] => 1.0
    [Content-Type] => text/plain; charset=UTF-8
    [Content-Transfer-Encoding] => 8bit
    [Plural-Forms] => nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 && n%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2);
    [X-Generator] => GlotPress/0.1
    [Project-Id-Version] => 3.5.x
)

Then I've exported translations as a .po file and verified that its headers were the same.

.mo files can also be decompiled back to .po with msgunfmt utility. That said, I'd agree that .po files should be downloaded as well.

#24 @boonebgorges
11 years ago

Opened #5030 for the move to load_plugin_textdomain()

#25 @boonebgorges
11 years ago

  • Milestone changed from 1.8 to Future Release

#26 @slaFFik
10 years ago

Sad, that this won't be in BP 2.0 :(

#27 follow-up: @boonebgorges
10 years ago

slaFFik - We've been told by the wp.org team that for any BP translation that is at 100%, updates will happen automatically on the first ping after upgrading to BP 2.0. Let us know if this happens :)

#28 in reply to: ↑ 27 @slaFFik
10 years ago

boonebgorges,
I will :) But as I understand, that ping won't happen if the translation is finished a day after the BuddyPress 2.0 release (but the user upgraded on the first day) - so the first ping from the user will be to the not finished translation and it will be ignore for now and then, am I right?

#29 @boonebgorges
10 years ago

But as I understand, that ping won't happen if the translation is finished a day after the BuddyPress 2.0 release (but the user upgraded on the first day) - so the first ping from the user will be to the not finished translation and it will be ignore for now and then, am I right?

No, I think this is incorrect. My understanding is that the translation checks will happen every time an installation pings the wordpress.org servers (as part of the normal checks for plugin, theme, core updates - every twelve hours, I think). If I upgrade to 2.0 tomorrow, and ru_RU's 2.0 branch is updated to 100% two days from now, then I'll receive the new translation 2-3 days from now. Basically, it's a wordpress.org-powered replacement for what this ticket was originally intended to do. (The details are still a bit unclear, because BP is one of a few plugins that is serving as a test case for the new system.)

#30 @slaFFik
10 years ago

Great! Thanks, Boone.

Last edited 10 years ago by slaFFik (previous) (diff)

#31 @slaFFik
10 years ago

So here is the report.

Russian is 100% translated. I had on own site BP 1.9.x, I clicked in admin area - Update the plugin. It was updated, but the translation was not downloaded. So, I guess more time needed to wait.

Placing translated files into /wp-content/languages/plugins with appropriate name doesn't work as well. Only the old-school /wp-content/plugins/buddypress/bp-languages/

Last edited 10 years ago by slaFFik (previous) (diff)

#32 @netweb
10 years ago

These translations updates should now be flowing, have been playing with various single and multisite tests this afternoon. I am getting translation updates for Akismet, bbPress and BuddyPress.

The updates delivered thus far have all been manual updates whilst I test them, I will setup a couple of test sites to see if they are installed with automatic updates for example:

I'll leave the sites alone and wait for some automatic update emails and see what gets updated.

#33 @DJPaul
9 years ago

  • Milestone changed from Future Release to 2.0
  • Resolution set to fixed
  • Status changed from reopened to closed

This was completed a long time again - setting an arbitrary milestone as I'm not sure exactly when it happened, and closing.

#34 @DJPaul
8 years ago

  • Component changed from Locale - i18n to I18N

#35 @DJPaul
8 years ago

  • Type changed from task to enhancement
Note: See TracTickets for help on using tickets.