Make WordPress Core

Opened 14 years ago

Closed 14 years ago

#15124 closed defect (bug) (fixed)

Script concatentation breaks l10n of javascript output in head

Reported by: macbrink's profile macbrink Owned by: westi's profile westi
Milestone: 3.1 Priority: high
Severity: major Version: 3.0.1
Component: JavaScript Keywords: convertentities, i18n, has-patch, needs-improvement
Focuses: Cc:

Description

This is what I find in the Admin header:

try{convertEntities(quicktagsL10n);}catch(e){};
/* ]]> */
</script>
<script type='text/javascript' src='http://myserver/wp/wp-admin/load-scripts.php?c=1&amp;load=jquery,utils,quicktags&amp;ver=b50ff5b9792a9e89a2e131ad3119a463'></script>

convertEntities() is called before the actual script has loaded.
This is quite annoying while debugging other scripts in firebug.

Change History (19)

#1 @macbrink
14 years ago

  • Component changed from General to JavaScript

#2 @westi
14 years ago

  • Milestone changed from Awaiting Review to 3.1
  • Owner set to westi
  • Status changed from new to accepted

#3 @westi
14 years ago

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

(In [15814]) Group the quicktags js. Fixes #15124.

#4 @macbrink
14 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Running WordPress 3.1-alpha with the patch.
Firebug breaks on error because

try{convertEntities(swfuploadL10n);}catch(e){};

is called before the utils javascript has loaded at opening the New Media screen.

#5 @westi
14 years ago

  • Summary changed from Convertentities called before script has loaded to Script concatentation breaks l10n of javascript output in head

Re-titling.

I think we need a better solution than just grouping everything in the footer.

#6 @westi
14 years ago

  • Keywords i18n added
  • Priority changed from normal to high
  • Severity changed from normal to major

(In [16000]) Remove the ghetto code and use the script loader properly on the login page.
Ensure that we actually have convertEntities available on the login page.
Introduce a login_footer action.
Hook in the script loader to the login_header and login_footer actions.
See #5919, #15124.

#7 @jane
14 years ago

  • Keywords has-patch needs-improvement added

#8 @westi
14 years ago

I really want to rewrite the Script loader and make it support grouping properly rather than the way it works at the moment.

But that is not something to do at this point in the cycle.

So I'm going to do something creative in that direction for now.

#9 @westi
14 years ago

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

(In [16282]) Move the l10n helper function into a seperate js file so we can always output it first.
Fixes #15124.

#10 @westi
14 years ago

Follow on ticket for future working #15381

#11 @westi
14 years ago

(In [16325]) Use the l10n helper js file in the installer. See #15124.

#12 @azizur
14 years ago

  • Cc azizur added

#13 @ocean90
14 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened
<script type='text/javascript' src='http://wp.local/wp-includes/js/l10n.dev.js?ver=20101110'></script> 
<script type='text/javascript' src='http://wp.local/wp-includes/js/l10n.dev.js?ver=20101110'></script>

Without SCRIPT_DEBUG:

<script type='text/javascript' src='http://wp.local/wp-includes/js/l10n.js?ver=20101110'></script> 
<script type='text/javascript' src='http://wp.local/wp-admin/load-scripts.php?c=1&amp;load=jquery,utils&amp;ver=7bb5fde83e0949a753f31e184d1c6ac7'></script> 
<script type='text/javascript' src='http://wp.local/wp-includes/js/l10n.js?ver=20101110'></script> 

Once too often.

#14 @westi
14 years ago

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

(In [16504]) Use do_items rather than do_item so the output of the l10n script is recorded. Fixes #15124.

#15 @azizur
14 years ago

Any reason why we'd need to always output on Non-Admin (i.e: Themes) in the header?

<script type='text/javascript' src='http://dev.site.com/wp-includes/js/l10n.js?ver=20101110'></script>

Like myself many theme developer would appreciate if this was disabled by default and API to enable it for those needed.

#16 @nacin
14 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#17 follow-up: @ocean90
14 years ago

azizur:
The reason is, that it's needed by the admin bar, see:

	$scripts->localize( 'admin-bar', 'adminBarL10n', array(
		'url' => __( 'URL:' ),
		'noShortlink' => __( 'No shortlink available for this page.' ),
		'l10n_print_after' => 'try{convertEntities(adminBarL10n);}catch(e){};',
	) );

#18 in reply to: ↑ 17 @westi
14 years ago

Replying to ocean90:

azizur:
The reason is, that it's needed by the admin bar, see:

	$scripts->localize( 'admin-bar', 'adminBarL10n', array(
		'url' => __( 'URL:' ),
		'noShortlink' => __( 'No shortlink available for this page.' ),
		'l10n_print_after' => 'try{convertEntities(adminBarL10n);}catch(e){};',
	) );

I suspect we may output it as the only js as well because of the way it works at the moment.

#19 @westi
14 years ago

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

I can't reproduce the output of this as the only JS file.

As ocean90 said we need this for l10n of JS strings.

Closing as Fixed.

Note: See TracTickets for help on using tickets.