Enable table captions; override CKE5's default downcast to generate <table><caption></table> instead of <figure><table><figcaption></figure>

Added this to the roadmap's Ensure CKE5 functionality matches that of Drupal core's CKE4 section at #3201824-33: Roadmap to core :)

This patch enables table captions.

Thank you for the patch @johnwebdev!

I tested the patch manually and I noticed that adding a caption generates a <figure> element with data-caption attribute. This is not consistent with CKEditor 4, which would simply use <caption> element inside the <table>. I think we want the CKEditor 4 behavior here so moving back to needs work.

Does the CKE5 table.TableCaption plugin support the configuration described in #4? We'll have to find out, and if not, we'll raise that in the upcoming meeting with the CKEditor 5 developers this Thursday :)

It seems like the caption is added to dataCaption attribute by the image plugin. When image plugin is disabled, the caption is added to figcaption and the <table> is wrapped with <figure>, which is still not what we'd expect.

I don't think their documentation include how we could make the caption downcast to the desired markup. However, that's something we should be able to do regardless of that.

Just asked the CKEditor team:

It’s surprising that https://ckeditor.com/docs/ckeditor5/latest/features/table.html#table-cap... in CKEditor 5 generates <figure><table>…</table><figcaption>this is the caption</figcaption></figure> instead of CKEditor 4's <table><caption>…this is the caption</caption><tbody></table> in https://ckeditor.com/docs/ckeditor4/latest/examples/table.html. I could not find docs about this, nor issue discussions in the public repo. Could you shine a light on this? 🙏

Agreed with #6 as the solution, but I'd rather first understand why they went this way.

@Anna_CKSource responded!

This is based on the research done before we started developing CKE5 and was summarized in https://ckeditor.github.io/editor-recommendations/features/table.html.

Some further references and discussions:

• https://github.com/ckeditor/ckeditor5/issues/3204#issuecomment-618289944
• https://github.com/ckeditor/ckeditor5/issues/3194#issuecomment-397234125

We'll need to investigate this.

Thanks, Anna! :)

Analysis

@Anna_CKSource provided two discussion links:

1. Over at https://github.com/ckeditor/ckeditor5/issues/3204#issuecomment-618289944, Alexander Schranz made the same remark. @Reinmar pointed to https://ckeditor.github.io/editor-recommendations/features/table.html for the rationale. But … that contains no discussion at all, just the conclusion, not the reasoning behind the conclusion. 😔
2. Then over at https://github.com/ckeditor/ckeditor5/issues/3194#issuecomment-397234125, there's very high-level description by Tomasz Jakut about a generic approach that is acceptable per the HTML 5 spec for captioning of arbitrary "self-contained content". This is fine, but still not an answer.
   Further in that same issue, Maciej asks at https://github.com/ckeditor/ckeditor5/issues/3194#issuecomment-397242600

   ps.: side question what about table > caption? It looks like another choice do describe table.

   — which is basically what we are asking here.

   Tomasz responded there:

   There are several techniques for describing table. In our case we use figcaption as also the mean to describe the whole figure element. Probably it would be nice to additionally bind figcaption with table using [aria-labelledby]/[aria-describedby].

   This is again not really an answer why they chose this approach. His link to https://html.spec.whatwg.org/multipage/tables.html#table-descriptions-te... is super valuable though: it shows there are 3 distinct WHATWG-approved techniques:

   1. Prose before the table → irrelevant here, that is always possible.
   2. table > caption → the current approach used in Drupal 8|9, because this is what CKEditor 4 used
   3. table > caption > details
   4. figure > table + figcaption → the approach used in CKEditor 5

Conclusion

My conclusion from this is that the CKEditor 5 development team understandably chose the approach that is generically applicable for all the markup they need to generate.

But Drupal has a different set of requirements:

1. it needs to care about not breaking existing sites' CSS/JS/PHP code
2. it prefers to stick to units of semantical HTML that can be transformed on output, through its Filter API

The only <figure> markup that Drupal currently generates, is for captioned images. Drupal stores <img src=… data-caption=…> and transforms this to <figure><img src=…><figcaption>…</figcaption></figure> via the caption filter. Why? Because this makes it easy for different channels/consumers of this data to present the data in a different way; they won't have to parse the full final HTML markup. Drupal prefers structured content, that is more canonical/less coupled to one channel/easier to transform.

Therefore it would IMHO still be preferable that Drupal overrides the ckeditor5-table's table.TableCaption's default downcast (both "data" and "editing" downcast).

Discussed during today's CKEditor 5 meeting.

@Reinmar confirms the above. He recommends to:

1. add a very late stage downcast override, to avoid Drupal having to repeat the same table construction logic
2. … but it should not be implemented as a downcast; there's a "view postfix" concept that would be a better fit
3. @Reinmar will provide more guidance on this in the near future

Upstream, the upcasting from <table><caption></table> will be taken care of upstream; they will need that for their customers upgrading from CKEditor 4 anyway.

We should have test coverage to ensure that a pasted <figure><table><figcaption></figure> while not using Full HTML does not allow the <figure> to continue to exist; it should be downcast to <table><caption></table>.

IOW: document the expected behavior through test coverage.

Per today's CKEditor 5 + Drupal meeting, both the upcast (actually, this is already working!) and downcast will be handled upstream, in https://github.com/ckeditor/ckeditor5/issues/10892.

#3269064: Update to CKEditor 5 v33.0.0 landed, this is now unblocked!

See #3269064-6: Update to CKEditor 5 v33.0.0 for details/next steps.

Even though we updated to CKEditor 5 v33.0.0, the DLL build doesn't contain it yet. They acknowledged this is a problem that they will fix. So this is again blocked on upstream: https://github.com/ckeditor/ckeditor5/issues/11516.

Thanks @bnjmnm for creating that issue upstream!

There should be a new CKEditor 5 release tomorrow. That means we should already start writing a test.

It seems like <table> element is now always wrapped with a <figure>. Bumping this to major based on that, even though this probably should be a critical since this is tagged as a stable blocker.

YAY!

Those first two bullets are indeed as expected 👍

Third bullet: do you mean when viewing a saved node inside CKEditor (i.e. editor downcast) or outside CKEditor (i.e. as visitor)?

1. Looks great — only nits! 🤓

2. +++ b/core/modules/ckeditor5/tests/src/FunctionalJavascript/TableTest.php
   @@ -0,0 +1,236 @@
   +    $this->adminUser = $this->drupalCreateUser([
   

   This doesn't need to be an "admin" user — a "content creator" is just fine!

3. +++ b/core/modules/ckeditor5/tests/src/FunctionalJavascript/TableTest.php
   @@ -0,0 +1,236 @@
   +      'administer filters',
   

   This permission is not needed.

4. +++ b/core/modules/ckeditor5/tests/src/FunctionalJavascript/TableTest.php
   @@ -0,0 +1,236 @@
   +    $this->drupalLogin($this->adminUser);
   

   This is the only place $this->adminUser is used — hence it does not need to be an object property.

5. +++ b/core/modules/ckeditor5/tests/src/FunctionalJavascript/TableTest.php
   @@ -0,0 +1,236 @@
   +   * Confirm tables convert to the expected markup.
   

   Übernit: s/Confirm/Confirms/

6. +++ b/core/modules/ckeditor5/tests/src/FunctionalJavascript/TableTest.php
   @@ -0,0 +1,236 @@
   +  public function assertTableStructureInRenderedPage() {
   

   Übernit: this and other methods could have the void return type.

+++ b/core/modules/ckeditor5/tests/src/FunctionalJavascript/TableTest.php
@@ -0,0 +1,234 @@
+  /**
+   * The user to use during testing.
+   *
+   * @var \Drupal\user\UserInterface
+   */
+  protected $adminUser;

This should still be removed. 🤓

+++ b/core/modules/ckeditor5/ckeditor5.ckeditor5.yml
@@ -445,6 +447,7 @@ ckeditor5_table:
+      - <caption>

👍

Looks great, thanks! 👍

Unrelated functional JS test failures for Layout Builder and Media → re-testing.

Committed 613b862 and pushed to 10.0.x. Also cherry-picked to 9.4.x and 9.3.x. Thanks!

Committed from 40000ft above the Labrador Sea on the way to DrupalCon Portland ✈️

🦅😄