Avoid changing default wpautop priority by brandonpayton · Pull Request #11722 · WordPress/gutenberg

brandonpayton · 2018-11-10T20:40:02Z

Description

This is a change to avoid problems due to changing the default wpautop priority.

How has this been tested?

Created a post in the classic editor and verified auto-embeds render correctly for the following raw content (mentioned in Embeds after breaks are broken #11691):

Some text above
https://www.youtube.com/watch?v=3V9QHBgrPNY

https://www.youtube.com/watch?v=f5KyMNDJE6o

Tested that block content opened in the classic editor retains its paragraphs.
Tested that non-block content is autop'd.
Unit tests.

Types of changes

Take a similar approach to core by removing the wpautop filter when block content is detected and subsequently restore it for later applications of the the_content filter.

The core changeset used for reference:
https://core.trac.wordpress.org/changeset/43879

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.

pento · 2018-11-11T23:25:31Z

 function gutenberg_wpautop( $content ) {
-	if ( has_blocks( $content ) ) {
-		return $content;
+	// If there are blocks in this content, we shouldn't run wpautop() on it later.


In Core, this block of code is run at the top of do_blocks(), there is no equivalent of gutenberg_wpautop(). Any particular reason why you chose to do it this way?

There's no good reason.

I may have made a mental mistake and avoided do_blocks because it is only defined conditionally by this plugin, but actually, we only need to worry about avoiding wpautop when we define do_blocks. Thanks for asking.

pento · 2018-11-11T23:25:40Z

+ *
+ * @access private
+ *
+ * @since 4.4


Suggested change

* @since 4.4

* @since 4.4.0

brandonpayton · 2018-11-12T22:38:14Z

+		$priority = has_filter( 'the_content', 'wpautop' );
+		if ( false !== $priority && doing_filter( 'the_content' ) && has_blocks( $content ) ) {
+			remove_filter( 'the_content', 'wpautop', $priority );
+			add_filter( 'the_content', 'wpautop', $priority + 1 );


Immediately re-adding wpautop here. 🤦‍♂️ Good example of why one shouldn't work during time set aside for recharging.

lkraav · 2018-11-15T22:11:42Z


+if ( ! function_exists( '_restore_wpautop_hook' ) ) {
+	/**
+	 * If do_blocks() needs to remove wp_autop() from the `the_content` filter,


wp_autop ~> wpautop

Thanks for catching that. It's fixed now.

catehstn · 2018-11-21T11:18:25Z

This looks like it's ready for another pass, @brandonpayton?

brandonpayton · 2018-11-21T17:20:37Z

This looks like it's ready for another pass, @brandonpayton?

Yep. It just needs a review.

brandonpayton · 2018-11-26T19:29:00Z

Hi @mtias, why are we changing this from a Bug to a Task? Loading Gutenberg in 4.9 breaks auto embeds as mentioned in #11691. That is buggy behavior.

noisysocks

Confirmed that this fixes #11691 👍

Could we include the unit test that's in https://core.trac.wordpress.org/changeset/43879?

Will be good to remove all of these workarounds once the Gutenberg plugin relies on having WordPress 5.0 installed.

noisysocks · 2018-11-26T23:45:18Z

This should go into the next plugin release. My understanding is that that will be 4.7, not 4.6.

mtias · 2018-11-28T11:50:18Z

@brandonpayton changed it to task because this is retrofitting the plugin with behaviour present in core, and since we are not doing another plugin release yet.

brandonpayton · 2018-11-28T19:51:32Z

ah, maybe I shouldn't be tagging PRs with "bug" at all. thanks for explaining @mtias

mtias · 2018-11-28T20:01:43Z

It's fine :) I just wanted to have a clear overview in the milestone to figure out what needs to be merged before release and what can be added after release but before 5.0.1, if that makes sense.

…rnmobile/danilo-try-to-fix-undo-redo * 'master' of https://github.com/WordPress/gutenberg: Avoid changing default wpautop priority (#11722) Try fixing the tab navigation issue (#12390) Polish editor style documentation (#12381)

brandonpayton added [Type] Bug An existing feature does not function as intended [Status] In Progress Tracking issues with work in progress labels Nov 10, 2018

brandonpayton added this to the 4.4 milestone Nov 10, 2018

brandonpayton self-assigned this Nov 10, 2018

pento reviewed Nov 11, 2018

View reviewed changes

brandonpayton force-pushed the fix/broken-wpautop-priority branch from e116708 to bd83c88 Compare November 12, 2018 21:12

brandonpayton commented Nov 12, 2018

View reviewed changes

brandonpayton removed the [Status] In Progress Tracking issues with work in progress label Nov 12, 2018

brandonpayton requested a review from a team November 12, 2018 23:09

youknowriad modified the milestones: 4.4, 4.5 Nov 15, 2018

lkraav reviewed Nov 15, 2018

View reviewed changes

brandonpayton force-pushed the fix/broken-wpautop-priority branch from ae40f6d to 46b6887 Compare November 16, 2018 18:57

mtias modified the milestones: 4.5, 4.6 Nov 19, 2018

Avoid changing default wpautop priority

e5ecd4a

brandonpayton force-pushed the fix/broken-wpautop-priority branch from 46b6887 to e5ecd4a Compare November 21, 2018 17:19

mtias added [Type] Task Issues or PRs that have been broken down into an individual action to take and removed [Type] Bug An existing feature does not function as intended labels Nov 22, 2018

noisysocks approved these changes Nov 26, 2018

View reviewed changes

youknowriad merged commit 2a66db0 into master Nov 29, 2018

youknowriad deleted the fix/broken-wpautop-priority branch November 29, 2018 12:48

youknowriad pushed a commit that referenced this pull request Nov 29, 2018

Avoid changing default wpautop priority (#11722)

b1fb5fc

Conversation

brandonpayton commented Nov 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How has this been tested?

Types of changes

Checklist:

Uh oh!

pento Nov 11, 2018

Choose a reason for hiding this comment

Uh oh!

brandonpayton Nov 12, 2018

Choose a reason for hiding this comment

Uh oh!

pento Nov 11, 2018

Choose a reason for hiding this comment

Uh oh!

brandonpayton Nov 12, 2018

Choose a reason for hiding this comment

Uh oh!

lkraav Nov 15, 2018

Choose a reason for hiding this comment

Uh oh!

brandonpayton Nov 16, 2018

Choose a reason for hiding this comment

Uh oh!

catehstn commented Nov 21, 2018

Uh oh!

brandonpayton commented Nov 21, 2018

Uh oh!

brandonpayton commented Nov 26, 2018

Uh oh!

noisysocks left a comment

Choose a reason for hiding this comment

Uh oh!

noisysocks commented Nov 26, 2018

Uh oh!

mtias commented Nov 28, 2018

Uh oh!

brandonpayton commented Nov 28, 2018

Uh oh!

mtias commented Nov 28, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

brandonpayton commented Nov 10, 2018 •

edited

Loading