Make WordPress Core

Opened 11 years ago

Closed 11 years ago

#34191 closed defect (bug) (fixed)

Shortcode JS Missed in r33118

Reported by: miqrogroove Owned by: johnbillion
Priority: normal Milestone: 4.4
Component: Shortcodes Version: 4.3
Severity: normal Keywords:
Cc: Focuses: javascript

Description

The shortcode attribute parser in Javascript no longer matches the PHP code.

Attachments (2)

34191.patch​ (753 bytes ) - added by miqrogroove 11 years ago.
34191.2.patch​ (2.6 KB ) - added by johnbillion 11 years ago.

Download all attachments as: .zip

Change History (11)

@miqrogroove
11 years ago

#1 @miqrogroove
11 years ago

  • Keywords has-patch commit added

#2 @johnbillion
11 years ago

  • Owner set to johnbillion
  • Status new → reviewing

#3 @johnbillion
11 years ago

  • Keywords needs-testing added; commit removed

34191.2.patch​ abstracts the regex into a function so it can be unit tested to ensure the JS and PHP versions remain consistent.

#4 @miqrogroove
11 years ago

The new test in 34191.2.patch​ passes for me. If I revert shortcode.js only, the new test fails as expected. Looks good to me.

Do we need to add this kind of test for the main regex as well?

#5 @johnbillion
11 years ago

  • Resolution → fixed
  • Status reviewing → closed

In 34933:

Abstract the shortcode attribute parsing regex into its own function, update the JavaScript counterpart, and introduce a test to ensure they do not diverge again.

Fixes #34191
Props miqrogroove, johnbillion

#6 @johnbillion
11 years ago

  • Keywords fixed-major added; has-patch needs-testing removed
  • Resolution fixed
  • Status closed → reopened

#7 @danielbachhuber
11 years ago

@miqrogroove Per discussion about adding documentation, shortcode.js (I believe) is used exclusively by mce views https://core.trac.wordpress.org/browser/tags/4.3/src/wp-includes/js/mce-view.js#L670.

I'm not sure how this would be best documented. Comment at the top of the file?

#8 @danielbachhuber
11 years ago

There are some existing shortcode parsing tests: https://core.trac.wordpress.org/browser/trunk/tests/qunit/wp-includes/js/shortcode.js

I'll update them to cover more cases.

#9 @johnbillion
11 years ago

  • Keywords fixed-major removed
  • Milestone 4.3.2 → 4.4
  • Resolution → fixed
  • Status reopened → closed
Note: See TracTickets for help on using tickets.