[wp-trac] [WordPress Trac] #45495: Extra P tags added to custom dynamic blocks

[WordPress Trac]

#45495: Extra P tags added to custom dynamic blocks
----------------------------------------------------+---------------------
 Reporter:  mattheu                                 |       Owner:  (none)
     Type:  defect (bug)                            |      Status:  new
 Priority:  normal                                  |   Milestone:  5.2
Component:  Editor                                  |     Version:
 Severity:  normal                                  |  Resolution:
 Keywords:  has-patch has-unit-tests needs-testing  |     Focuses:
----------------------------------------------------+---------------------
Changes (by pento):

 * keywords:  has-patch needs-unit-tests => has-patch has-unit-tests needs-
     testing


Comment:

 For folks following the ticket, @aldavigdis and I have been brainstorming
 a bit on this.

 So, the problem appears to be when a dynamic block callback run the
 `the_content` filter. @mattheu's example does so when it calls
 `get_the_excerpt()`, and the feature that @aldavigdis is working on
 (Jetpack's related posts feature) does so, too.

 It doesn't matter if `the_content` is being run on the same post that
 originally triggered the `do_blocks()` call. The important part is that
 it's being run a second time before the first finishes.

 Here's the order of proceedings:

 * `the_content` filter is started.
  * `do_blocks()` is called.
   * `wpautop()` is removed from `the_content`.
   * `_restore_wpautop_hook()` is added to `the_content`.
   * The dynamic block callback is called.
    * The dynamic block callback triggers an inner run of `the_content`
 filter.
     * `do_blocks()` is called.
      * `wpautop` isn't registered, so it isn't removed in this inner call.
      * `do_blocks()` returns.
     * `_restore_wpautop_hook()`, having been added to `the_content` by the
 outer `do_blocks()` call, is run.
      * `wpautop()` is re-added to `the_content`.
      * `_restore_wpautop_hook()` is removed from `the_content`.
     * The inner `the_content` filter finishes.
    * The dynamic block callback returns.
   * The outer `do_blocks()` call returns.
  * Having been added by the inner `the_content` run, `wpautop()` is
 called.
  * Having been removed by the inner `the_content` run,
 `_restore_wpautop_hook()` ''is not'' called.
  * The outer `the_content` run finishes.

 Provided the dynamic block callback doesn't start a different filter, this
 could also triggered by the dynamic block callback calling `do_blocks()`
 directly.

 I suspect that @aldavigdis' patch is the correct way to tackle this
 problem. [attachment:"45495.diff"] includes a unit test showing this
 particular behaviour.

 Things I'm not yet certain of:
 * Are there side effects to this fix which aren't accounted for?
 * Are there plugins with workarounds in place for this bug? If so, how
 will they be affected? If they just remove `wpautop` themselves, that
 should be fine: it'll be re-added at the end by outer `the_content`
 filter.
 * Shortcodes expect to have `wpautop()` run on their inner content. Does
 this change things?
 * What happens with classic blocks?

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/45495#comment:7>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform