Make WordPress Core

Opened 17 years ago

Closed 17 years ago

#3911 closed defect (bug) (fixed)

Javascript should not be localized by via PHP

Reported by: mdawaffe's profile mdawaffe Owned by: mdawaffe's profile mdawaffe
Milestone: 2.2 Priority: high
Severity: normal Version: 2.1.2
Component: Optimization Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description (last modified by foolswisdom)

Example: list-manipulation-js.php has to load all of WordPress in order to localize just a two strings.

We can extend the script loader class to include a JS snippet that overwrites the default strings.

#BB605

Attachments (1)

3911.diff (15.7 KB) - added by mdawaffe 17 years ago.

Download all attachments as: .zip

Change History (8)

@mdawaffe
17 years ago

#1 @mdawaffe
17 years ago

  • Keywords has-patch 2nd-opinion added
  • Owner changed from anonymous to mdawaffe
  • Status changed from new to assigned

3911.diff
To apply

  1. svn mv wp-includes/js/list-manipulation-js.php wp-includes/js/list-manipulation.js

Features

  1. wp_localize_script( script_name, JS object_name, array( JS val => localized string ) See listman example in new script-loader.php

#2 @ryan
17 years ago

Nice

#3 @majelbstoat
17 years ago

+1 for this. At the moment I have to resort to defining variables in admin_head() and wp_head() which is ugly to say the least.

What object will the passed variables be attached to? (window, or a some other?) It seems to be magically attached to tempobj somehow...?

And in your patch, I didn't quite see what

 - if ( this.theList.childNodes.length % 2 )
 + if ( ( this.theList.childNodes.length + this.altOffset ) % 2 )


did, or whether it was part of the changes?

#4 @mdawaffe
17 years ago

majelbstoat,

The variables get put into an object you specify. The patch on this ticket tells the script loader to add them to an object called "listManL10n":

wp_localize_script( 'listman', 'listManL10n', array( 
   'jumpText' => __('Jump to new item'), 
   'delText' => __('Are you sure you want to delete this %s?') 
) );

(in the patch the above is actually accomplished by a line that looks like $this->localize( ... );)

Script loader then generates the following output in the page's HEAD.

<script type='text/javascript'>
/* <![CDATA[ */
	listManL10n = {
		jumpText: "Jump to new item",
		delText: "Are you sure you want to delete this %s?"
	}
/* ]]> */
</script>

It's the responsibility of the JavaScript to then go find it. listMan was tweaked to do just that with the following lines (included in the patch).

if ( 'undefined' != typeof listManL10n ) 
   Object.extend(listMan.prototype, listManL10n);

As to your other question:

Oops, forgot about the offset stuff. I think it fixes a coloring bug that sometimes shows up when AJAX adding new items to a list with alternating colors, but that's been sitting around locally on my machine for a while....

#5 @majelbstoat
17 years ago

Sweet, I didn't spot the listManL10n object. That's a nice system and will make localisation a lot easier (kind of a special interest of mine).

#6 @foolswisdom
17 years ago

  • Description modified (diff)

#7 @ryan
17 years ago

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

(In [4968]) JS localization from mdawaffe. fixes #3911

Note: See TracTickets for help on using tickets.