WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#15124 closed defect (bug) (fixed)

Script concatentation breaks l10n of javascript output in head

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

  • Component changed from General to JavaScript

#2 @westi
7 years ago

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

#3 @westi
7 years ago

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

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

#4 @macbrink
7 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
7 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
7 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
7 years ago

  • Keywords has-patch needs-improvement added

#8 @westi
7 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
7 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
7 years ago

Follow on ticket for future working #15381

#11 @westi
7 years ago

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

#12 @azizur
7 years ago

  • Cc azizur added

#13 @ocean90
7 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
7 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
7 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
7 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#17 follow-up: @ocean90
7 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
7 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
7 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.