Make WordPress Core

Opened 9 years ago

Closed 9 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 9 years ago.
27907.2.patch (1.1 KB) - added by Clorith 9 years ago.
Also check if the accidental newlines are preceding the shortcode
27907.qunit.patch (1.0 KB) - added by Clorith 9 years ago.
qunit tests for catching newlines in escaped shortcodes
27907.diff (2.0 KB) - added by jorbin 9 years ago.
27907.2.diff (3.6 KB) - added by jorbin 9 years ago.
27907.3.patch (3.5 KB) - added by Clorith 9 years ago.
27907.4.diff (3.1 KB) - added by gcorne 9 years ago.

Download all attachments as: .zip

Change History (21)

#1 @Clorith
9 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.


9 years ago

#3 @SergeyBiryukov
9 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
9 years ago

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

@Clorith
9 years ago

#5 @Clorith
9 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
9 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.


9 years ago

@Clorith
9 years ago

qunit tests for catching newlines in escaped shortcodes

@jorbin
9 years ago

#7 @jorbin
9 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.


9 years ago

#9 @nacin
9 years ago

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

@jorbin
9 years ago

@Clorith
9 years ago

#10 @Clorith
9 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
9 years ago

#11 @gcorne
9 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
9 years ago

  • Keywords commit added; 2nd-opinion removed

#13 @nacin
9 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
9 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.