[drupalMedia] Support captions on <drupal-media>

Moved alignment to #3260554: [drupalMedia] Support alignment on <drupal-media> because it can be worked separately from captions.

The issue summary also mentions data-view-mode. I think we should open another new issue for that.

Need rebase at least, currently looking at the code

Caught #3263601: Follow-up, fix JS error by fixing form ID selector while reviewing this

At render time this is adding a figcaption element in the frontend without a parent figure element. This is not valid HTML5, spec says it's

<figure>
  <!-- something something -->
  <figcaption></figcaption>
</figure>

// OR

<figure>
  <figcaption></figcaption>
  <!-- something something -->
</figure>

At the moment it's only adding a caption below the image, not sure we want to add more settings to set the caption above or below the image.

As for the validity of the HTML generated I'm guessing this is out of scope. Why are media wrapped in an <article> tag and not <figure>, because the purpose of the figure element seems to match the use we have for embedding media in the body of an article. My mistake, it does it properly.

This is not the case inside the CKEditor instance though: we have

<div class="" contenteditable="false" data-entity-type="media" data-entity-uuid="">
  <article>
    <img>
  </article>
  <figcaption data-placeholder="Enter media caption" contenteditable="true"></figcaption>
</div>

Here the wrapping div should be a figure tag no? that's the case for ckeditor image plugin.

Good point that we need to change the wrapping <div> to <figure> on editing view. Somehow missed that as I was working on this 🤦‍♂️

I'm wondering if we should open a follow-up for figuring out how to configure whether the caption preview should render before or after. We might want to do this for both, <drupal-media> and <img>.

The placement of the caption is definitely out of scope for this issue. It's not available in the upstream imagecaption plugin so might make sense to open something upstream first? and see how they do it so we can just copy/paste :D

Thanks for the explanations lauriii, this looks good to me now.

@Wim Leers I've added similar test coverage that works around the limitations we have on selecting text in CKEditor. Wondering if you think we still need the follow-up after this?

Opened follow-up for a bug I discovered with this MR: #3264775: [drupalMedia] Toolbar should be visible when element inside <drupalMedia> is focused.

Switching to NW due to a failing test that is part of the MR and not one of those randoms we run into at inopportune times.

BTW I'm not getting the test failure locally, sharing the CI output since it took a very time to load and I want to save people that step:

Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testEditableCaption
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'Llamas are the most awesome ever'
+'Llamas are the most awesome ever'

/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Equality/IsEqual.php:96
/var/www/html/core/modules/ckeditor5/tests/src/FunctionalJavascript/MediaTest.php:371
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:726

That was indeed failing 9/10 times on DrupalCI... I think I managed to fix that. I ran this 50x on #2488258-195: [ignore] Patch testing issue.

D10 reroll, working on 9.4

D9

There was a small problem with the CSS caused by the reroll in #31 which I fixed on MR and here's that again as patches.

Very nice to see Media captions! Excellent addition.

Committed to 10.0.x/9.4.x and cherry-picked to 9.3.x as CKEditor 5 is experimental.