WordPress.org

Make WordPress Core

#19830 closed defect (bug) (fixed)

wp_localize_script() combines statements improperly

Reported by: TobiasBg Owned by: azaozz
Priority: normal Milestone: 3.4
Component: General Version: 3.3
Severity: minor Keywords: easy-fix has-patch commit
Cc:

Description

When making multiple calls to wp_localize_script() on the same $handle, the individual JSONified objects are merged together with ;\n in this line.
However, each individual var statement has already been correctly terminated by a semicolon ; a few lines above.

Due to that, the resulting output has an extra semicolon after each but the last var statement (i.e. ;;).

Removing the semicolon from the string in line 151 should fix this.

Attachments (2)

semicolonfix.diff (584 bytes) - added by tw2113 17 months ago.
semicolonfix1.diff (584 bytes) - added by tw2113 17 months ago.
Fixed placement of the semicolon

Download all attachments as: .zip

Change History (10)

comment:1 scribu17 months ago

  • Owner set to azaozz
  • Severity changed from normal to minor
  • Status changed from new to assigned

comment:2 azaozz17 months ago

Generally having two consecutive semicolons in JS is acceptable (not only at the end of line). Many JS libraries still add a semicolon at the very beginning (when minimized) to prevent concatenation errors.

The second semicolon could be moved to L:146 as it matters only when there's 'l10n_print_after' part.

comment:3 TobiasBg17 months ago

Yes, that's true, as the two semicolons are not causing any issues/warnings/errors. It's just looking weird, and with just one semicolon the code looks cleaner (I feel).

Moving it from line 151 to 146 looks good to me, as the only time when the semicolon could matter would be, if the semicolon at the end of the legacy l10n_print_after is missing.

comment:4 SergeyBiryukov17 months ago

  • Keywords easy-fix added
  • Milestone changed from Awaiting Review to 3.4

tw211317 months ago

comment:5 follow-up: TobiasBg17 months ago

@tw2113: Thanks for the patch! The moved semicolon should go after the $after variable though, as that variable is the one that we need to "protect" from not having a semicolon. Everything before $after will already have one from line 143.

tw211317 months ago

Fixed placement of the semicolon

comment:6 in reply to: ↑ 5 tw211317 months ago

  • Keywords has-patch added; needs-patch removed

You're welcome. Fixed with the new diff file.

Replying to TobiasBg:

@tw2113: Thanks for the patch! The moved semicolon should go after the $after variable though, as that variable is the one that we need to "protect" from not having a semicolon. Everything before $after will already have one from line 143.

comment:7 SergeyBiryukov17 months ago

  • Keywords commit added

comment:8 azaozz17 months ago

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

In [19832]:

Move an unsightly semicolon to where it belongs in wp_localize_script(), props tw2113, fixes #19830

Note: See TracTickets for help on using tickets.