Add needed attributes to kses allowed tags for the Gallery block

[notnownikki]

Description

The Gallery block uses data-id and data-link on the images,
and for non-admin users, kses strips these out, which makes Gutenberg
error when loading a post with a Gallery that's been authored by a non-admin
user.

This PR adds a filter function that adds the attributes we need.

How has this been tested?

Log in with a user with the author role.
Make a new post, add a gallery block.
Save the post.
Reload the editor.

The gallery block should load and be editable as expected.

Types of changes

Bug fix

Checklist:

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

[mcsf]

It's too bad that we have to explicitly allow given data attributes, rather than accept all data-*. It begs the question: are we doing things wrong in Gallery? Is WordPress/KSES approaching this the wrong way with a blanket exclusion of data attributes?

[notnownikki]

My opinion is that data-* should be allowed, and WordPress/KSES is doing it wrong when it comes to those attributes.

But others might know better...

[mcsf]

We can patch this specifically for Gallery, but if so we need a proper solution in the very near future. I also wonder if this a change that can be effected directly in core as part of 5.0 work. Pinging @pento, curious if you have thoughts.

[aduth]

Is WordPress/KSES approaching this the wrong way with a blanket exclusion of data attributes?

For that matter, I'm not totally sure what's being accomplished by stripping any attribute. Without considering its value, I can't see any security-related issue with an attribute presence alone.

[pento]

I can't think of anything off the top of my head, I suspect it's just something that hasn't been added: KSES is built on an allow list of individual attributes, adding a wildcard would require a little more care to implement.

I'm going to float it with the security team, but it's probably going to take some time to explore and create a patch, so let's just go with this PR to fix the immediate issue for now.

[pento]

@notnownikki: For testing this, it'd be good to have a test that ran over each *.serialized.html file in the /test/integration/full-content/fixtures directory, and ran it through wp_unslash( wp_filter_post_kses( wp_slash( $html ) ) ), to check that other attributes aren't being removed.

class-parsing-test.php has similar functionality that you could copy/pasta. 😉

[notnownikki]

@pento that sounds like a good idea. Will get that added to this PR!

[notnownikki]

@pento ok, I wrote the test, and we have other failures. I'd like to get this in to 3.9 and address the other failures in a different PR. Some of the failures are just simple spacing fails, but some are aria attributes that kses is stripping out, which I don't think affects the block loading/saving functionality like it does with the gallery, but will take more time to go through and fix.

[mcsf]

@notnownikki: that seems fair, given the circumstances. Seems like we've uncovered something bigger, especially considering a11y in WordPress. :)

As for the spacing, that might be OK: we already have some clever diffing with isEquivalentHTML et al., we could expand on that.

[notnownikki]

@mcsf fantastic, I'm working on the new test now, there are a few elements getting stripped out that affect bits of functionality. PR coming when they're fixed!

[pento]

I was a little curious about whether the test would uncover similar issues. 😉

Thank you for diving into this, @notnownikki!