Make WordPress Core

Opened 12 years ago

Closed 10 years ago

#19666 closed defect (bug) (fixed)

Switching from HTML to Visual editor removes some line breaks inside PRE

Reported by: mrclay's profile mrclay Owned by:
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.3
Component: Editor Keywords: has-patch
Focuses: Cc:

Description

All line breaks inside PRE elements used to transmit correctly when switching editor mode from HTML to Visual. Around WP3.1 or so, line breaks started to be removed if they happened to be inside an inline element. The common use case is a single CODE element in a PRE element:

<pre><code>Line 1
Line 2
Line 3</code></pre>

The above markup used to be handled correctly when switching between editor modes in either direction, but now the line breaks are converted to spaces when switching from HTML to Visual mode.

Many users have posts with a single CODE inside PRE, as it's semantically correct for source code, and is the convention required to support some syntax highlighters like Chili.

Attachments (1)

19666.patch (1.2 KB) - added by mrclay 12 years ago.
patch for issue 19666: /wp-includes/class-wp-editor.php

Download all attachments as: .zip

Change History (14)

#1 @SergeyBiryukov
12 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

#2 @mrclay
12 years ago

  • Version set to 3.3

This does not appear to be a duplicate of #15367, but it is rather caused by a new TinyMCE bug you can see in this fiddle. I walked through editor.dev.js in a debugger to verify it's handling the markup correctly.

I'm reporting it to TinyMCE now, but I think this will certainly cause problems for up-to-date WP users in the meantime.

#3 @SergeyBiryukov
12 years ago

  • Milestone set to Awaiting Review
  • Resolution duplicate deleted
  • Status changed from closed to reopened

#4 @mrclay
12 years ago

TinyMCE bug filed. As I noted there, this behavior might be due to the improvements in [16394]. Namely, if the JS version of wpautop was incorrectly converting PRE newlines to BRs before switching to the Visual editor, these BRs were working around the TinyMCE bug.

@mrclay
12 years ago

patch for issue 19666: /wp-includes/class-wp-editor.php

#5 follow-up: @mrclay
12 years ago

19666.patch adds an onBeforeSetContent handler to TinyMCE that converts newlines to BRs inside PREs just before content is added to the editor. You can see how this fixes the behavior in this fiddle. wp-admin/js/editor.js removes these before POSTing or switching editors, so I think this should not add side-effects.

BTW I was able to fix this during switching in editor.js, but this still did not cover the case of when the Visual editor is initialized on page load. The tinyMCE event handler covers both cases.

#6 @SergeyBiryukov
12 years ago

  • Keywords has-patch added

#7 in reply to: ↑ 5 @azaozz
12 years ago

Replying to mrclay:

19666.patch adds an onBeforeSetContent handler to TinyMCE...

Yes, ideally this should be fixed upstream but we could add a workaround for now. The place for this would be inside the "wordpress" MCE plugin. We shouldn't use the 'init.setup = function()...` method as it could be overwritten by another WP plugin.

#8 follow-up: @mrclay
12 years ago

I'll look for that and submit another patch. Maybe plugins shouldn't be allowed to set "setup" and instead have to use a hook to add code to the setup function body. Just a thought.

#9 in reply to: ↑ 8 @azaozz
12 years ago

Replying to mrclay:

I'll look for that and submit another patch.

No need for patch, it's just copy/paste into /tinymce/plugins/wordpress/editor_plugin.js.

Maybe plugins shouldn't be allowed to set "setup" and instead have to use a hook to add code to the setup function body. Just a thought.

Was thinking about that too. Perhaps it's better to encourage plugins to use "real" TinyMCE plugins instead of using init.setup. As far as I've seen it's rarely used at the moment. Adding an external MCE plugin is very easy in WP.

Last edited 12 years ago by azaozz (previous) (diff)

#10 @cgrymala
12 years ago

  • Cc curtiss@… added

#11 @rahul286
11 years ago

  • Cc rahul286@… added

#12 @Bueltge
11 years ago

  • Cc frank@… added

#13 @azaozz
10 years ago

  • Milestone changed from Awaiting Review to 3.9
  • Resolution set to fixed
  • Status changed from reopened to closed

Fixed in TinyMCE 4.0, see #24067.

Note: See TracTickets for help on using tickets.