Reusable block list having additional lines added

[karmatosed]

This is a weird bug, I am not even sure how replicable but I got it several times.

I was writing a blog post and had a reusable block. It was a list and on auto saving this happened:

That extra line I did not add :/ Most odd and unfortunately I couldn't work out the cause of this so posting in hope we can dig a bit. I was using Chrome at the time.

[noisysocks]

Steps to reproduce:

1. Convert a list to a reusable block
2. Open the inserter

---

It looks like this is due to a bug in the list block.

When a list block is serialize()d, it generates markup that looks like this:

<!-- wp:list -->
<ul>
    <li>A</li>
    <li>B</li>
</ul>
<!-- /wp:list -->

Note that the markup is nicely indented. When we later parse() this markup, we get attributes that look like this:

values: {
    0: '    ',
    1: { type: 'li', props: { children: 'A' } ... },
    2: '    ',
    3: { type: 'li', props: { children: 'B' } ... },
    4: '    ',
}

Note the empty values: they're there because we've sourced from children which picks up both the text nodes and element nodes that exist as children of the <ul>.

This isn't ordinarily a problem because we pass values into Editable.defaultValue which is smart enough to consider only element nodes. But, when a reusable block is updated, we call editor.setContent( content, { format: 'raw' } ) (see #667). Since we're disabling TinyMCE's sanitisation, the text nodes aren't filtered out and so we end up with empty <li>s.

There's a few ways we can fix this:

1. We could call editor.setContent( content ) instead of editor.setContent( content, { format: 'raw' } ).
2. We could change the core/list selector to select child elements instead of child nodes
   1. This could be done by adding a flag to the children matcher, e.g.:

      attributes: {
          values: {
              type: 'array',
              source: 'children',
              selector: 'ol,ul',
              filterElements: true, // <-- selects only nodes with node.nodeType === NODE_ELEMENT
              default: [],
          },
      },

   2. Or, this could be done by using the query matcher in a way similar to how core/quote works, e.g.:

      attributes: {
          value: {
              type: 'array',
              source: 'query',
              selector: 'body > ol > *, body > ul > *',
              query: {
                  children: 'element',
              },
              default: [],
          },
      },

@aduth: why do we disable TinyMCE's sanitisation? Do you have a preference on which way we fix this?

[aduth]

@aduth: why do we disable TinyMCE's sanitisation? Do you have a preference on which way we fix this?

If I recall correctly, it was just to ensure that we were working with as raw a markup as possible, assuming that what's generated from and into markup by React is what we want to use; obviously we hadn't accounted for prettified HTML. There's also a bit of overhead in using the non-raw content setters / getters. Lastly, I believe we're currently passing 'raw' as the argument to getContent (where performance could matter a bit more), so likely this way just for consistency's sake.

Off the top of my head, I don't see any issues that would discourage us from setting content without { format: 'raw' }