Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#27907 closed defect (bug) (fixed)

Shortcode double brackets ending on newline breaks post editor

Reported by: clorith's profile Clorith Owned by: gcorne's profile gcorne
Milestone: 3.9.1 Priority: normal
Severity: major Version: 3.9
Component: TinyMCE Keywords: has-patch commit
Focuses: administration Cc:

Description

When showing off a shortcode you for example by use of [[gallery]], the post editor breaks if you do the final ] on a new line.

A quick typo can result in this. By updating or publishing the post in this state the editor appears blank, you can see an extended bit of HTML in Text view, but the visual editor remains blank.

Changes made in the visual editor will not be saved, the post is essentially "broken" at this point.

JS error causing it Uncaught TypeError: Cannot call method 'slice' of undefined found in shortcode.js on Line 40.

Attachments (7)

27907.patch (825 bytes) - added by Clorith 10 years ago.
27907.2.patch (1.1 KB) - added by Clorith 10 years ago.
Also check if the accidental newlines are preceding the shortcode
27907.qunit.patch (1.0 KB) - added by Clorith 10 years ago.
qunit tests for catching newlines in escaped shortcodes
27907.diff (2.0 KB) - added by jorbin 10 years ago.
27907.2.diff (3.6 KB) - added by jorbin 10 years ago.
27907.3.patch (3.5 KB) - added by Clorith 10 years ago.
27907.4.diff (3.1 KB) - added by gcorne 10 years ago.

Download all attachments as: .zip

Change History (21)

#1 @Clorith
10 years ago

  • Summary changed from Shortcode double brackerts ending on newline breaks to Shortcode double brackerts ending on newline breaks post editor

This ticket was mentioned in IRC in #wordpress-dev by Clorith. View the logs.


10 years ago

#3 @SergeyBiryukov
10 years ago

  • Summary changed from Shortcode double brackerts ending on newline breaks post editor to Shortcode double brackets ending on newline breaks post editor

#4 @ocean90
10 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 3.9.1

@Clorith
10 years ago

#5 @Clorith
10 years ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed

Attempting a solution for this by stopping the attempt at slicing if there is no result.match.

As the bug only appears to affect shortcodes with in-editor rendering, this would allow the user to easily spot it and edit accordingly as preventing this slicing makes the shortcode render (which would be the correct behavior, as it's not a properly escaped shortcode to begin with).

@Clorith
10 years ago

Also check if the accidental newlines are preceding the shortcode

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.


10 years ago

@Clorith
10 years ago

qunit tests for catching newlines in escaped shortcodes

@jorbin
10 years ago

#7 @jorbin
10 years ago

I consolidated the two patches into one and fixed a jshint error in the new unit test.

This ticket was mentioned in IRC in #wordpress-dev by nacin1. View the logs.


10 years ago

#9 @nacin
10 years ago

  • Owner set to gcorne
  • Status changed from new to assigned

@jorbin
10 years ago

@Clorith
10 years ago

#10 @Clorith
10 years ago

I took the combined patch @jorbin made with the additional unit tests and changed the js fix slightly (the original had an awkward double comma I hadn't spotted for the comments).

I think it's better to check if the match is defined once instead of checking it with each if statement as well, it's the little things.

@gcorne
10 years ago

#11 @gcorne
10 years ago

I spent some time reviewing the existing patches and reading through the code, and I am fairly confident that the underlying issue was what amounts to a couple typos. In 27907.4.diff, I fixed the typos and adjusted the new tests. I found both the old and the new tests a little tricky to follow because of how they test only result.index, which might be something worth revisiting.

#12 @nacin
10 years ago

  • Keywords commit added; 2nd-opinion removed

#13 @nacin
10 years ago

In 28223:

Shortcode JS: Avoid errors when an escaped shortcode includes a newline between brackets.

props gcorne.
props Clorith, jorbin for tests.
see #27907, for trunk.

#14 @nacin
10 years ago

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

In 28224:

Shortcode JS: Avoid errors when an escaped shortcode includes a newline between brackets.

Merges [28223] to the 3.9 branch.

props gcorne.
props Clorith, jorbin for tests.
fixes #27907.

Note: See TracTickets for help on using tickets.