Fix the Table of Contents links of headings with inline formatting

The anchor of a heading was built from the text returned by
`serializeNodesToText`, which trims every leaf of the node and joins them
with whitespace, because it is meant for indexing. A heading that Slate
splits into several leafs, either by inline formatting or by an inline
link, therefore produced an anchor that did not match the one the Table
of Contents block linked to, and the entry led nowhere.

Build the anchor from the text of the heading as it is rendered instead,
and report it as the third element of the `tocEntry` of the block, so
that the Table of Contents block links to the anchor that the block
actually renders, instead of slugging its stored plaintext. Blocks that
report no anchor keep the previous behavior.

Closes #8340

Author: Somilg11

packages/volto-slate/news/8340.bugfix

@@ -0,0 +1 @@
+Fixed the anchor of a heading being built from its indexed plaintext, which broke it when the heading contained inline formatting or a link. The anchor is now built from the text of the heading as it is rendered, and is also reported as the third element of the block's `tocEntry`. @Somilg11

packages/volto-slate/src/blocks/Text/TextBlockView.jsx

@@ -1,11 +1,7 @@
-import {
-  serializeNodes,
-  serializeNodesToText,
-} from '@plone/volto-slate/editor/render';
+import { serializeNodes } from '@plone/volto-slate/editor/render';
+import { getAnchor } from '@plone/volto-slate/utils/toc';
 import config from '@plone/volto/registry';
 import isEqual from 'lodash/isEqual';
-import Slugger from 'github-slugger';
-import { normalizeString } from '@plone/volto/helpers/Utils/Utils';
 
 const TextBlockView = (props) => {
   const { id, data, styling = {} } = props;
@@ -17,9 +13,7 @@ const TextBlockView = (props) => {
     const res = { ...styling };
     if (node.type && isEqual(path, [0])) {
       if (topLevelTargetElements.includes(node.type) || override_toc) {
-        const text = serializeNodesToText(node?.children || []);
-        const slug = Slugger.slug(normalizeString(text));
-        res.id = slug || id;
+        res.id = getAnchor(node) || id;
       }
     }
     return res;

packages/volto-slate/src/blocks/Text/TextBlockView.test.jsx

@@ -0,0 +1,79 @@
+import React from 'react';
+import renderer from 'react-test-renderer';
+import config from '@plone/volto/registry';
+import TextBlockView from './TextBlockView';
+import { getAnchor } from '@plone/volto-slate/utils/toc';
+
+beforeAll(() => {
+  config.settings = {
+    slate: {
+      topLevelTargetElements: ['h2', 'h3'],
+      elements: {
+        default: ({ attributes, children }) => (
+          <p {...attributes}>{children}</p>
+        ),
+        h2: ({ attributes, children }) => <h2 {...attributes}>{children}</h2>,
+        link: ({ attributes, children }) => <a {...attributes}>{children}</a>,
+      },
+      leafs: {
+        italic: ({ children }) => <em>{children}</em>,
+      },
+    },
+  };
+});
+
+const getHeadingId = (value) => {
+  const component = renderer.create(
+    <TextBlockView id="block-id" data={{ value }} />,
+  );
+  return component.root.findByType('h2').props.id;
+};
+
+test('renders the anchor of a plain heading', () => {
+  expect(
+    getHeadingId([
+      { type: 'h2', children: [{ text: 'Subtitle 1: Everything is okay' }] },
+    ]),
+  ).toBe('subtitle-1-everything-is-okay');
+});
+
+test('renders the same anchor for a heading with inline formatting', () => {
+  const value = [
+    {
+      type: 'h2',
+      children: [
+        { text: 'Subtitle 2: Everything is ' },
+        { text: 'not', italic: true },
+        { text: ' okay' },
+      ],
+    },
+  ];
+  expect(getHeadingId(value)).toBe('subtitle-2-everything-is-not-okay');
+  // The table of contents entry of the block links to this anchor.
+  expect(getHeadingId(value)).toBe(getAnchor(value[0]));
+});
+
+test('renders the same anchor for a heading containing a link', () => {
+  const value = [
+    {
+      type: 'h2',
+      children: [
+        { text: 'Subtitle 3: Everything is ' },
+        {
+          type: 'link',
+          data: { url: '/some-page' },
+          children: [{ text: 'okay' }],
+        },
+        { text: '' },
+      ],
+    },
+  ];
+  expect(getHeadingId(value)).toBe('subtitle-3-everything-is-okay');
+  expect(getHeadingId(value)).toBe(getAnchor(value[0]));
+});
+
+test('falls back to the block id for an empty heading', () => {
+  expect(getHeadingId([{ type: 'h2', children: [{ text: '' }] }])).toBe(
+    'block-id',
+  );
+});

packages/volto-slate/src/blocks/Text/index.jsx

@@ -20,6 +20,7 @@ import {
   cancelEsc,
 } from './keyboard';
 import { splitAtSeam } from './keyboard/splitAtSeam';
+import { getAnchor, getAnchorText } from '@plone/volto-slate/utils/toc';
 import { withDeleteSelectionOnEnter } from '@plone/volto-slate/editor/extensions';
 import {
   breakList,
@@ -129,10 +130,17 @@ export default function applyConfig(config) {
     tocEntry: (block = {}) => {
       const { value, override_toc, entry_text, level, plaintext } = block;
       const type = value?.[0]?.type;
+      // The anchor is computed from the current value, and not from the stored
+      // plaintext, so that it always matches the `id` rendered by the view.
+      const anchor = getAnchor(value?.[0]);
       return override_toc && level
-        ? [parseInt(level.slice(1)), entry_text]
+        ? [parseInt(level.slice(1)), entry_text, anchor]
         : config.settings.slate.topLevelTargetElements.includes(type)
-          ? [parseInt(type.slice(1)), plaintext]
+          ? [
+              parseInt(type.slice(1)),
+              getAnchorText(value?.[0]) || plaintext,
+              anchor,
+            ]
           : null;
     },
   };

packages/volto-slate/src/utils/index.js

@@ -9,3 +9,4 @@ export * from './random';
 export * from './selection';
 export * from './volto-blocks';
 export * from './mime-types';
+export * from './toc';

packages/volto-slate/src/utils/toc.js

@@ -0,0 +1,51 @@
+import { Text } from 'slate';
+import Slugger from 'github-slugger';
+import { normalizeString } from '@plone/volto/helpers/Utils/Utils';
+
+/**
+ * Get the text of a node as it reads when rendered.
+ *
+ * Unlike `serializeNodesToText`, which is meant for indexing and separates the
+ * text of every node with whitespace, this concatenates the text of the inline
+ * children of a node without inserting or dropping any whitespace. This is
+ * required to build anchors, since a heading split into several leafs by inline
+ * formatting (bold, italic, links) must produce the same text as the same
+ * heading written without any formatting.
+ *
+ * @function serializeNodeToInlineText
+ * @param {Object} node A Slate node.
+ * @returns {string} The text of the node.
+ */
+export const serializeNodeToInlineText = (node) => {
+  if (!node) return '';
+  if (Text.isText(node)) return node.text;
+  return (node.children || []).map(serializeNodeToInlineText).join('');
+};
+
+/**
+ * Get the anchor text of a node, with the surrounding whitespace removed and
+ * the inner whitespace collapsed.
+ *
+ * The empty text nodes that Slate keeps around inline elements would otherwise
+ * end up as stray separators in the resulting slug.
+ *
+ * @function getAnchorText
+ * @param {Object} node A Slate node.
+ * @returns {string} The anchor text of the node.
+ */
+export const getAnchorText = (node) =>
+  serializeNodeToInlineText(node).replace(/\s+/g, ' ').trim();
+
+/**
+ * Get the anchor (the `id` attribute) that the view of a Slate block renders
+ * for the given node.
+ *
+ * Both the block view and the block's table of contents entry use it, so that
+ * the links of the table of contents always match the anchors in the page.
+ *
+ * @function getAnchor
+ * @param {Object} node A Slate node.
+ * @returns {string} The anchor of the node.
+ */
+export const getAnchor = (node) =>
+  Slugger.slug(normalizeString(getAnchorText(node)));

packages/volto-slate/src/utils/toc.test.js

@@ -0,0 +1,90 @@
+import { getAnchor, getAnchorText, serializeNodeToInlineText } from './toc';
+
+const plainHeading = {
+  type: 'h2',
+  children: [{ text: 'Subtitle 1: Everything is okay' }],
+};
+
+const italicHeading = {
+  type: 'h2',
+  children: [
+    { text: 'Subtitle 2: Everything is ' },
+    { text: 'not', italic: true },
+    { text: ' okay' },
+  ],
+};
+
+const partiallyItalicWordHeading = {
+  type: 'h2',
+  children: [
+    { text: 'Subtitle 3: n' },
+    { text: 'o', italic: true },
+    { text: 't okay' },
+  ],
+};
+
+const linkHeading = {
+  type: 'h2',
+  children: [
+    { text: 'Subtitle 4: Everything is ' },
+    {
+      type: 'link',
+      data: { url: '/some-page' },
+      children: [{ text: 'okay' }],
+    },
+    { text: '' },
+  ],
+};
+
+describe('serializeNodeToInlineText', () => {
+  it('returns the text of a node without a type', () => {
+    expect(serializeNodeToInlineText({ text: 'Hello' })).toBe('Hello');
+  });
+
+  it('does not add whitespace between the leafs of a node', () => {
+    expect(serializeNodeToInlineText(partiallyItalicWordHeading)).toBe(
+      'Subtitle 3: not okay',
+    );
+  });
+
+  it('includes the text of inline elements', () => {
+    expect(serializeNodeToInlineText(linkHeading)).toBe(
+      'Subtitle 4: Everything is okay',
+    );
+  });
+
+  it('returns an empty string for an empty node', () => {
+    expect(serializeNodeToInlineText(undefined)).toBe('');
+    expect(serializeNodeToInlineText({ type: 'h2' })).toBe('');
+  });
+});
+
+describe('getAnchorText', () => {
+  it('collapses the inner whitespace and trims the outer one', () => {
+    expect(
+      getAnchorText({
+        type: 'h2',
+        children: [{ text: '  Some  ' }, { text: '' }, { text: 'heading ' }],
+      }),
+    ).toBe('Some heading');
+  });
+});
+
+describe('getAnchor', () => {
+  it('slugs a heading', () => {
+    expect(getAnchor(plainHeading)).toBe('subtitle-1-everything-is-okay');
+  });
+
+  it('is not affected by inline formatting', () => {
+    expect(getAnchor(italicHeading)).toBe('subtitle-2-everything-is-not-okay');
+    expect(getAnchor(partiallyItalicWordHeading)).toBe('subtitle-3-not-okay');
+  });
+
+  it('is not affected by inline links', () => {
+    expect(getAnchor(linkHeading)).toBe('subtitle-4-everything-is-okay');
+  });
+
+  it('returns an empty string for an empty heading', () => {
+    expect(getAnchor({ type: 'h2', children: [{ text: '' }] })).toBe('');
+  });
+});

packages/volto/cypress/tests/core/blocks/block-anchors.js

@@ -114,4 +114,51 @@ describe('Block Tests: Anchors', () => {
     ).click();
     cy.get('h2[id="title-2-u-a"]').scrollIntoView().should('be.visible');
   });
+
+  it('Add Block: add content to TOC with inline formatting', () => {
+    // Change page title
+    cy.clearSlateTitle();
+    cy.getSlateTitle().type('Slate Heading Anchors with inline formatting');
+    cy.getSlate().click();
+
+    // Add TOC block
+    cy.get('.ui.basic.icon.button.block-add-button').first().click();
+    cy.get(".blocks-chooser .ui.form .field.searchbox input[type='text']").type(
+      'table of contents',
+    );
+    cy.get('.button.toc').click();
+
+    // Add a heading, and make one of its words italic
+    cy.get('.ui.basic.icon.button.block-add-button').first().click();
+    cy.get(".blocks-chooser .ui.form .field.searchbox input[type='text']").type(
+      'text',
+    );
+    cy.get('.button.slate').click();
+    cy.get('.ui.drag.block.inner.slate')
+      .click()
+      .type('Title 1 is not okay')
+      .click();
+    cy.get('.ui.drag.block.inner.slate span span span').setSelection(
+      'Title 1 is not okay',
+    );
+    cy.get('.slate-inline-toolbar .button-wrapper a[title="Title"]').click({
+      force: true,
+    });
+    cy.setSlateSelection('not');
+    cy.clickSlateButton('Italic');
+
+    // Save page
+    cy.get('#toolbar-save').click();
+    cy.url().should('eq', Cypress.config().baseUrl + '/my-page');
+
+    // The anchor of the heading ignores the inline formatting, and the TOC
+    // entry links to it
+    cy.get('h2[id="title-1-is-not-okay"] em').contains('not');
+    cy.get(
+      `.table-of-contents a[href="${subpathPrefix}/my-page#title-1-is-not-okay"]`,
+    ).click();
+    cy.get('h2[id="title-1-is-not-okay"]')
+      .scrollIntoView()
+      .should('be.visible');
+  });
 });

packages/volto/news/8340.bugfix

@@ -0,0 +1 @@
+Fixed the links of the `Table of Contents` block breaking when a heading contains inline formatting or a link. The block now uses the anchor reported by the block that owns the heading, instead of slugging its indexed plaintext. @Somilg11

packages/volto/src/components/manage/Blocks/ToC/View.jsx

@@ -55,6 +55,7 @@ export const getBlocksTocEntries = (properties, tocData) => {
       const i = `${id}-${index}`;
       const level = entry[0];
       const title = entry[1];
+      const anchor = entry[2];
       const items = [];
       if (!level || !levels.includes(level)) return;
       tocEntriesLayout.push(i);
@@ -63,6 +64,7 @@ export const getBlocksTocEntries = (properties, tocData) => {
         title: title || block.plaintext,
         items,
         id: i,
+        anchor,
       };
       if (level < rootLevel) {
         rootLevel = level;
@@ -119,6 +121,7 @@ const View = (props) => {
       if (entry) {
         const level = entry[0];
         const title = entry[1];
+        const anchor = entry[2];
         const items = [];
         if (!title?.trim() && !block.plaintext?.trim()) return;
         if (!level || !levels.includes(level)) return;
@@ -130,6 +133,7 @@ const View = (props) => {
           id,
           override_toc: block.override_toc,
           plaintext: block.plaintext,
+          anchor,
         };
       }
     });