Make WordPress Core

Opened 6 years ago

Last modified 6 years ago

#44617 new defect (bug)

Introduce new 'hasBlock' behavior for TinyMCE 'BeforeSetContent' and 'SaveContent' events

Reported by: danielbachhuber's profile danielbachhuber Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Editor Keywords: has-patch dev-feedback
Focuses: Cc:

Description

When TinyMCE editor content has Gutenberg blocks in it, we can assume <p> paragraph tags are included too. In this scenario, wp.editor.autop() should not be applied to the content.

However, even when wp.editor.autop() isn't applied, TinyMCE applies <p> paragraph tags to all root nodes (e.g. the content of Shortcode Blocks). We can mitigate this behavior by stripping <p> paragraph tags from Shortcode Blocks on the 'SaveContent' event.

From wordpress/gutenberg#4672

Attachments (1)

44617.1.diff (2.4 KB) - added by danielbachhuber 6 years ago.

Download all attachments as: .zip

Change History (9)

#1 @danielbachhuber
6 years ago

  • Keywords dev-feedback added

To test 44617.1.diff:

  1. Revert https://github.com/WordPress/gutenberg/pull/2708 locally (probably on a test branch).
  1. Create a post in the Classic Editor with the following:
<!-- wp:shortcode -->
[column]

I daresay that Fry has discovered the smelliest object in the known universe! For the last time, I don't like lilacs! Your 'first' wife was the one who liked lilacs! No argument here. A true inspiration for the children.

[/column]
<!-- /wp:shortcode -->

<!-- wp:paragraph -->
<p>It has a normal paragraph block</p>
<!-- /wp:paragraph -->

<!-- wp:shortcode -->
[my-awesome-shortcode]
<!-- /wp:shortcode -->
  1. Apply the 44617.1.diff patch, run grunt build, switch to the Visual editor, and force refresh.
  1. Switch back to the Text editor and observe the following:
<!-- wp:shortcode -->[column] I daresay that Fry has discovered the smelliest object in the known universe! For the last time, I don't like lilacs! Your 'first' wife was the one who liked lilacs! No argument here. A true inspiration for the children. [/column]<!-- /wp:shortcode -->

<!-- wp:paragraph --><p>It has a normal paragraph block</p><!-- /wp:paragraph -->

<!-- wp:shortcode -->[my-awesome-shortcode]<!-- /wp:shortcode -->

You'll note newlines are stripped, which I'm undecided as to whether is still a bug. At the very least, the paragraph tags in the Paragraph Block persist, and no paragraph tags are applied to the Shortcode Blocks.

#2 follow-up: @azaozz
6 years ago

TinyMCE applies <p> paragraph tags to all root nodes...

Right. There is even a setting for that (which tag to use).

Couple of things:

  1. This is "not supported formatting" in the current (classic) editor:
    [column]
    
    I daresay that Fry has discovered the smelliest object in the known universe! For the last time, I don't like lilacs! Your 'first' wife was the one who liked lilacs! No argument here. A true inspiration for the children.
    
    [/column]
    

Frankly I don't see why we should add support for it now. That will also most likely introduce several edge cases which will have to be fixed at the cost of adding other edge cases, etc. etc.

  1. The current/classic editor is set to remove all line breaks when "saving" the content. This is done to facilitate running pre_wpautop() to remove the <p> tags (at that point line breaks become meaningful in the content). Don't think we should change that, the amount of back-compat and edge cases will be overwhelming.

We discussed the possible solutions to shortcodes in existing content when loading them in Gutenberg blocks:

  • One possibility is that the content will have to be run through wpautop() and then, if there are any shorcodes in it, the content will also have to be run through shortcodes_unautop(). That is the exact handling on the front-end, and will ensure all shortcodes "arrive" in proper formatting to Gutenberg.
  • Another possibility would be to run shortcodes_unautop() even on posts created with Gutenberg when there are old-style shortcodes in them. (This may actually be the better option, needs testing).

Don't think we need to specifically deal with old-style shortcodes that are "improperly formatted" then saved in a Gutenberg post and then edited in the classic editor. The only change that might make sense would be to check if a post has shortcodes in it, and still run shortcodes_unautop() even when wpautop() is disabled.

#3 in reply to: ↑ 2 ; follow-up: @danielbachhuber
6 years ago

Replying to azaozz:

  1. This is "not supported formatting" in the current (classic) editor:

Frankly I don't see why we should add support for it now. That will also most likely introduce several edge cases which will have to be fixed at the cost of adding other edge cases, etc. etc.

I'm not sure it matters whether it's "supported" vs."not supported". It works, to some degree, in the Classic Editor. If we change the behavior in Gutenberg, then some number of users will notice and report the issue as a bug.

Also, the problem applies to the single-line Shortcode Block too, not just multi-line shortcodes. TinyMCE currently generates this:

<!-- wp:shortcode -->
<p>[my-awesome-shortcode]</p>
<!-- /wp:shortcode -->

We discussed the possible solutions to shortcodes in existing content when loading them in Gutenberg blocks:

There's some more up-to-date conversation in https://github.com/WordPress/gutenberg/issues/3900#issuecomment-406490577

#4 in reply to: ↑ 3 @azaozz
6 years ago

Replying to danielbachhuber:

...If we change the behavior in Gutenberg,

Hm, may be missing something but we are talking here about changing the behaviour in the current/classic editor (not Gutenberg) and possibly breaking things, introducing hard to fix edge cases, etc.

Also, the problem applies to the single-line Shortcode Block too, not just multi-line shortcodes. TinyMCE currently generates this:

The classic editor has always generated exactly this. There haven't been any changes there for at least 5 years :)

Thunk I wasn't very clear above :) What I'm proposing is to keep doing the same things we've been doing since... shortcodes were introduced: run the content through the regex that removes extra <p> tags. This is in the shortcodes_unautop() function. If it needs to be implemented in Gutenberg too (although I don't see why), we should "convert" it to js.

What happens then you run the above example code through shortcodes_unautop()? :)

This ticket was mentioned in Slack in #core-editor by danielbachhuber. View the logs.


6 years ago

This ticket was mentioned in Slack in #core by jon_bossenger. View the logs.


6 years ago

#7 @pento
6 years ago

  • Milestone changed from 4.9.8 to Future Release

Moving this out of 4.9.8 for now.

I think we need to clean up some of Classic's behaviour when the user opens a block-based post, but that's not critical for 4.9.8.

For now, we'll add a warning when a block-based post is opened in Classic, this can be done via Gutenberg, we can evaluate merging it to Core in a future 4.9.x release.

Note: See TracTickets for help on using tickets.