Fix some edge cases with Content Editor serializing

- Also allow rendering block content in tables behind
  a feature flag

Changelog: fixed

app/assets/javascripts/content_editor/extensions/table_cell.js

 import { TableCell } from '@tiptap/extension-table-cell';
+import { isBlockTablesFeatureEnabled } from '../services/feature_flags';
 
 export default TableCell.extend({
-  content: 'inline*',
+  content: isBlockTablesFeatureEnabled() ? 'block+' : 'inline*',
 });

app/assets/javascripts/content_editor/extensions/table_header.js

 import { TableHeader } from '@tiptap/extension-table-header';
+import { isBlockTablesFeatureEnabled } from '../services/feature_flags';
 
 export default TableHeader.extend({
-  content: 'inline*',
+  content: isBlockTablesFeatureEnabled() ? 'block+' : 'inline*',
 });

app/assets/javascripts/content_editor/services/feature_flags.js

+export function isBlockTablesFeatureEnabled() {
+  return gon.features?.contentEditorBlockTables;
+}

app/assets/javascripts/content_editor/services/markdown_serializer.js

@@ -30,6 +30,12 @@ import TableRow from '../extensions/table_row';
 import TaskItem from '../extensions/task_item';
 import TaskList from '../extensions/task_list';
 import Text from '../extensions/text';
+import {
+  renderHardBreak,
+  renderTable,
+  renderTableCell,
+  renderTableRow,
+} from './serialization_helpers';
 
 const defaultSerializerConfig = {
   marks: {
@@ -65,6 +71,7 @@ const defaultSerializerConfig = {
       expelEnclosingWhitespace: true,
     },
   },
+
   nodes: {
     [Blockquote.name]: defaultMarkdownSerializer.nodes.blockquote,
     [BulletList.name]: defaultMarkdownSerializer.nodes.bullet_list,
@@ -80,7 +87,7 @@ const defaultSerializerConfig = {
 
       state.write(`:${name}:`);
     },
-    [HardBreak.name]: defaultMarkdownSerializer.nodes.hard_break,
+    [HardBreak.name]: renderHardBreak,
     [Heading.name]: defaultMarkdownSerializer.nodes.heading,
     [HorizontalRule.name]: defaultMarkdownSerializer.nodes.horizontal_rule,
     [Image.name]: (state, node) => {
@@ -95,60 +102,10 @@ const defaultSerializerConfig = {
     [Reference.name]: (state, node) => {
       state.write(node.attrs.originalText || node.attrs.text);
     },
-    [Table.name]: (state, node) => {
-      state.renderContent(node);
-    },
-    [TableCell.name]: (state, node) => {
-      state.renderInline(node);
-    },
-    [TableHeader.name]: (state, node) => {
-      state.renderInline(node);
-    },
-    [TableRow.name]: (state, node) => {
-      const isHeaderRow = node.child(0).type.name === 'tableHeader';
-
-      const renderRow = () => {
-        const cellWidths = [];
-
-        state.flushClose(1);
-
-        state.write('| ');
-        node.forEach((cell, _, i) => {
-          if (i) state.write(' | ');
-
-          const { length } = state.out;
-          state.render(cell, node, i);
-          cellWidths.push(state.out.length - length);
-        });
-        state.write(' |');
-
-        state.closeBlock(node);
-
-        return cellWidths;
-      };
-
-      const renderHeaderRow = (cellWidths) => {
-        state.flushClose(1);
-
-        state.write('|');
-        node.forEach((cell, _, i) => {
-          if (i) state.write('|');
-
-          state.write(cell.attrs.align === 'center' ? ':' : '-');
-          state.write(state.repeat('-', cellWidths[i]));
-          state.write(cell.attrs.align === 'center' || cell.attrs.align === 'right' ? ':' : '-');
-        });
-        state.write('|');
-
-        state.closeBlock(node);
-      };
-
-      if (isHeaderRow) {
-        renderHeaderRow(renderRow());
-      } else {
-        renderRow();
-      }
-    },
+    [Table.name]: renderTable,
+    [TableCell.name]: renderTableCell,
+    [TableHeader.name]: renderTableCell,
+    [TableRow.name]: renderTableRow,
     [TaskItem.name]: (state, node) => {
       state.write(`[${node.attrs.checked ? 'x' : ' '}] `);
       state.renderContent(node);
@@ -175,7 +132,7 @@ const wrapHtmlPayload = (payload) => `<div>${payload}</div>`;
  * that parses the Markdown and converts it into HTML.
  * @returns a markdown serializer
  */
-export default ({ render = () => null, serializerConfig }) => ({
+export default ({ render = () => null, serializerConfig = {} } = {}) => ({
   /**
    * Converts a Markdown string into a ProseMirror JSONDocument based
    * on a ProseMirror schema.

app/assets/javascripts/content_editor/services/serialization_helpers.js

+import { uniq } from 'lodash';
+import { isBlockTablesFeatureEnabled } from './feature_flags';
+
+const defaultAttrs = {
+  td: { colspan: 1, rowspan: 1, colwidth: null },
+  th: { colspan: 1, rowspan: 1, colwidth: null },
+};
+
+const tableMap = new WeakMap();
+
+function shouldRenderCellInline(cell) {
+  if (cell.childCount === 1) {
+    const parent = cell.child(0);
+    if (parent.type.name === 'paragraph' && parent.childCount === 1) {
+      const child = parent.child(0);
+      return child.isText && child.marks.length === 0;
+    }
+  }
+
+  return false;
+}
+
+function getRowsAndCells(table) {
+  const cells = [];
+  const rows = [];
+  table.descendants((n) => {
+    if (n.type.name === 'tableCell' || n.type.name === 'tableHeader') {
+      cells.push(n);
+      return false;
+    }
+
+    if (n.type.name === 'tableRow') {
+      rows.push(n);
+    }
+
+    return true;
+  });
+  return { rows, cells };
+}
+
+function getChildren(node) {
+  const children = [];
+  for (let i = 0; i < node.childCount; i += 1) {
+    children.push(node.child(i));
+  }
+  return children;
+}
+
+function shouldRenderHTMLTable(table) {
+  const { rows, cells } = getRowsAndCells(table);
+
+  const cellChildCount = Math.max(...cells.map((cell) => cell.childCount));
+  const maxColspan = Math.max(...cells.map((cell) => cell.attrs.colspan));
+  const maxRowspan = Math.max(...cells.map((cell) => cell.attrs.rowspan));
+
+  const rowChildren = rows.map((row) => uniq(getChildren(row).map((cell) => cell.type.name)));
+  const cellTypeInFirstRow = rowChildren[0];
+  const cellTypesInOtherRows = uniq(rowChildren.slice(1).map(([type]) => type));
+
+  // if the first row has headers, and there are no headers anywhere else, render markdown table
+  if (
+    !(
+      cellTypeInFirstRow.length === 1 &&
+      cellTypeInFirstRow[0] === 'tableHeader' &&
+      cellTypesInOtherRows.length === 1 &&
+      cellTypesInOtherRows[0] === 'tableCell'
+    )
+  ) {
+    return true;
+  }
+
+  if (cellChildCount === 1 && maxColspan === 1 && maxRowspan === 1) {
+    // if all rows contain only one paragraph each and no rowspan/colspan, render markdown table
+    const children = uniq(cells.map((cell) => cell.child(0).type.name));
+    if (children.length === 1 && children[0] === 'paragraph') {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+function openTag(state, tagName, attrs) {
+  let str = `<${tagName}`;
+
+  str += Object.entries(attrs || {})
+    .map(([key, value]) => {
+      if (defaultAttrs[tagName]?.[key] === value) return '';
+
+      return ` ${key}=${state.quote(value?.toString() || '')}`;
+    })
+    .join('');
+
+  return `${str}>`;
+}
+
+function closeTag(state, tagName) {
+  return `</${tagName}>`;
+}
+
+function isInBlockTable(node) {
+  return tableMap.get(node);
+}
+
+function isInTable(node) {
+  return tableMap.has(node);
+}
+
+function setIsInBlockTable(table, value) {
+  tableMap.set(table, value);
+
+  const { rows, cells } = getRowsAndCells(table);
+  rows.forEach((row) => tableMap.set(row, value));
+  cells.forEach((cell) => {
+    tableMap.set(cell, value);
+    if (cell.childCount && cell.child(0).type.name === 'paragraph')
+      tableMap.set(cell.child(0), value);
+  });
+}
+
+function unsetIsInBlockTable(table) {
+  tableMap.delete(table);
+
+  const { rows, cells } = getRowsAndCells(table);
+  rows.forEach((row) => tableMap.delete(row));
+  cells.forEach((cell) => {
+    tableMap.delete(cell);
+    if (cell.childCount) tableMap.delete(cell.child(0));
+  });
+}
+
+function renderTagOpen(state, tagName, attrs) {
+  state.ensureNewLine();
+  state.write(openTag(state, tagName, attrs));
+}
+
+function renderTagClose(state, tagName, insertNewline = true) {
+  state.write(closeTag(state, tagName));
+  if (insertNewline) state.ensureNewLine();
+}
+
+function renderTableHeaderRowAsMarkdown(state, node, cellWidths) {
+  state.flushClose(1);
+
+  state.write('|');
+  node.forEach((cell, _, i) => {
+    if (i) state.write('|');
+
+    state.write(cell.attrs.align === 'center' ? ':' : '-');
+    state.write(state.repeat('-', cellWidths[i]));
+    state.write(cell.attrs.align === 'center' || cell.attrs.align === 'right' ? ':' : '-');
+  });
+  state.write('|');
+
+  state.closeBlock(node);
+}
+
+function renderTableRowAsMarkdown(state, node, isHeaderRow = false) {
+  const cellWidths = [];
+
+  state.flushClose(1);
+
+  state.write('| ');
+  node.forEach((cell, _, i) => {
+    if (i) state.write(' | ');
+
+    const { length } = state.out;
+    state.render(cell, node, i);
+    cellWidths.push(state.out.length - length);
+  });
+  state.write(' |');
+
+  state.closeBlock(node);
+
+  if (isHeaderRow) renderTableHeaderRowAsMarkdown(state, node, cellWidths);
+}
+
+function renderTableRowAsHTML(state, node) {
+  renderTagOpen(state, 'tr');
+
+  node.forEach((cell, _, i) => {
+    const tag = cell.type.name === 'tableHeader' ? 'th' : 'td';
+
+    renderTagOpen(state, tag, cell.attrs);
+
+    if (!shouldRenderCellInline(cell)) {
+      state.closeBlock(node);
+      state.flushClose();
+    }
+
+    state.render(cell, node, i);
+    state.flushClose(1);
+
+    renderTagClose(state, tag);
+  });
+
+  renderTagClose(state, 'tr');
+}
+
+export function renderTableCell(state, node) {
+  if (!isBlockTablesFeatureEnabled()) {
+    state.renderInline(node);
+    return;
+  }
+
+  if (!isInBlockTable(node) || shouldRenderCellInline(node)) {
+    state.renderInline(node.child(0));
+  } else {
+    state.renderContent(node);
+  }
+}
+
+export function renderTableRow(state, node) {
+  if (isInBlockTable(node)) {
+    renderTableRowAsHTML(state, node);
+  } else {
+    renderTableRowAsMarkdown(state, node, node.child(0).type.name === 'tableHeader');
+  }
+}
+
+export function renderTable(state, node) {
+  if (isBlockTablesFeatureEnabled()) {
+    setIsInBlockTable(node, shouldRenderHTMLTable(node));
+  }
+
+  if (isInBlockTable(node)) renderTagOpen(state, 'table');
+
+  state.renderContent(node);
+
+  if (isInBlockTable(node)) renderTagClose(state, 'table');
+
+  // ensure at least one blank line after any table
+  state.closeBlock(node);
+  state.flushClose();
+
+  if (isBlockTablesFeatureEnabled()) {
+    unsetIsInBlockTable(node);
+  }
+}
+
+export function renderHardBreak(state, node, parent, index) {
+  const br = isInTable(parent) ? '<br>' : '\\\n';
+
+  for (let i = index + 1; i < parent.childCount; i += 1) {
+    if (parent.child(i).type !== node.type) {
+      state.write(br);
+      return;
+    }
+  }
+}

app/controllers/projects/wikis_controller.rb

@@ -5,5 +5,9 @@ class Projects::WikisController < Projects::ApplicationController
 
   alias_method :container, :project
 
+  before_action do
+    push_frontend_feature_flag(:content_editor_block_tables, @project, default_enabled: :yaml)
+  end
+
   feature_category :wiki
 end

config/feature_flags/development/content_editor_block_tables.yml

+---
+name: content_editor_block_tables
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66187
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338937
+milestone: '14.3'
+type: development
+group: group::editor
+default_enabled: false

spec/frontend/fixtures/api_markdown.yml

@@ -102,14 +102,10 @@
   markdown: |-
     | header | header |
     |--------|--------|
-    | cell | cell |
-    | cell | cell |
-- name: table_with_alignment
-  markdown: |-
-    | header | : header : | header : |
-    |--------|------------|----------|
-    | cell | cell | cell |
-    | cell | cell | cell |
+    | `code` | cell with **bold** |
+    | ~~strike~~ | cell with _italic_ |
+
+    # content after table
 - name: emoji
   markdown: ':sparkles: :heart: :100:'
 - name: reference