WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#3911 closed defect (bug) (fixed)

Javascript should not be localized by via PHP

Reported by: mdawaffe Owned by: 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 7 years ago.

Download all attachments as: .zip

Change History (8)

mdawaffe7 years ago

comment:1 mdawaffe7 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

comment:2 ryan7 years ago

Nice

comment:3 majelbstoat7 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?

comment:4 mdawaffe7 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....

comment:5 majelbstoat7 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).

comment:6 foolswisdom7 years ago

  • Description modified (diff)

comment:7 ryan7 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.