VPR-210 fix(cms): keep legacy CMS tables borderless

DESIGN.md

@@ -212,7 +212,7 @@ A UC Davis institutional palette: Aggie Blue is the foundation, Aggie Gold is th
 - **Ink** (`ink`): Quasar `dark`; default high-contrast text. `dark-page` is the dark page background.
 - **Body Grey** (`body-grey`): The AA-safe muted text color, `--ucdavis-black-60`. Quasar's default `.text-grey` and `.bg-grey` are remapped to this so muted text still clears 4.5:1.
 - **Surface** (`surface`): Card, panel, and workspace background, and the welcome card over the hero photo.
-- **Table Header** (`table-header`): Sticky `q-table` header fill, and the fill on `q-table__top` and `q-table__bottom`.
+- **Table Header** (`table-header`): Sticky `q-table` header fill, the fill on `q-table__top` and `q-table__bottom`, and the header fill on bordered tables in CMS content.
 - **Gold Text** (`gold-text`): The darkened gold used for gold-colored *text* on light backgrounds, since bright gold fails AA at text sizes.
 - **Focus Blue** (`focus-blue`): The outer ring of the keyboard focus halo in the app. The only non-brand hue in the system.
 - **Splash Card Ink** (`splash-card-ink`) and **Splash Card Muted** (`splash-card-muted`): Body and secondary text inside the white sign-in card. Slightly warmer and softer than the workspace pairing, because the card sits on a photograph rather than on a page.

VueApp/src/components/RichTextEditor.vue

@@ -39,7 +39,7 @@ import EditorLinkDialog from "@/components/editor/EditorLinkDialog.vue"
 import EditorImageDialog from "@/components/editor/EditorImageDialog.vue"
 import EditorTableDialog from "@/components/editor/EditorTableDialog.vue"
 import { buildImageHtml, buildLinkHtml, buildTableHtml, parseLinkHref } from "@/components/editor/editor-html"
-import type { LinkKind } from "@/components/editor/editor-html"
+import type { LinkKind, TableOptions } from "@/components/editor/editor-html"
 
 /**
  * Shared rich-text (HTML) editor wrapping Quasar's QEditor. Centralizes the accessibility and
@@ -309,7 +309,7 @@ function onImageSubmit(value: { src: string; alt: string }) {
     void closeThenRun(imageDialogOpen, savedRange, "insertHTML", buildImageHtml(value))
 }
 
-function onTableSubmit(value: { rows: number; cols: number; header: boolean }) {
+function onTableSubmit(value: TableOptions) {
     void closeThenRun(tableDialogOpen, savedRange, "insertHTML", buildTableHtml(value))
 }
 

VueApp/src/components/__tests__/editor-html.test.ts

@@ -121,7 +121,7 @@ test("buildImageHtml makes src relative and escapes alt, emitting alt even when
 test("buildTableHtml with a header splits header row from body rows", () => {
     const html = buildTableHtml({ rows: 3, cols: 3, header: true })
     expect(html).toBe(
-        "<table><thead><tr><th>&nbsp;</th><th>&nbsp;</th><th>&nbsp;</th></tr></thead>" +
+        '<table border="1"><thead><tr><th>&nbsp;</th><th>&nbsp;</th><th>&nbsp;</th></tr></thead>' +
             "<tbody><tr><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td></tr>" +
             "<tr><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td></tr></tbody></table><p><br></p>",
     )
@@ -135,10 +135,15 @@ test("buildTableHtml with rows=1 and header=true has no tbody at all", () => {
 
 test("buildTableHtml with rows=1 and header=false has a single body row and no thead", () => {
     const html = buildTableHtml({ rows: 1, cols: 2, header: false })
-    expect(html).toBe("<table><tbody><tr><td>&nbsp;</td><td>&nbsp;</td></tr></tbody></table><p><br></p>")
+    expect(html).toBe('<table border="1"><tbody><tr><td>&nbsp;</td><td>&nbsp;</td></tr></tbody></table><p><br></p>')
     expect(html).not.toContain("<thead")
 })
 
+test("buildTableHtml writes border=0 and the align attribute when asked", () => {
+    const html = buildTableHtml({ rows: 1, cols: 1, header: false, border: false, align: "center" })
+    expect(html).toBe('<table border="0" align="center"><tbody><tr><td>&nbsp;</td></tr></tbody></table><p><br></p>')
+})
+
 test("buildTableHtml clamps cols to 20", () => {
     const html = buildTableHtml({ rows: 1, cols: 99, header: false })
     expect(html.match(/<td>/gu) ?? []).toHaveLength(20)

VueApp/src/components/__tests__/rich-text-editor.test.ts

@@ -5,13 +5,15 @@ import RichTextEditor from "@/components/RichTextEditor.vue"
 import EditorImageDialog from "@/components/editor/EditorImageDialog.vue"
 import EditorLinkDialog from "@/components/editor/EditorLinkDialog.vue"
 import EditorTableDialog from "@/components/editor/EditorTableDialog.vue"
+import RecordFormDialog from "@/components/RecordFormDialog.vue"
 import { MAX_TABLE_COLS } from "@/components/editor/editor-html"
+import type { TableAlign } from "@/components/editor/editor-html"
 // <script setup> exposes nothing to the type system, so the tests reach each dialog's refs through
 // a narrowed view of the instance instead of `any`.
 type LinkDialogVm = { address: string; text: string; newWindow: boolean }
 type LinkKind = "url" | "email" | "phone"
 type ImageDialogVm = { file: File | null; alt: string }
-type TableDialogVm = { rows: number; cols: number }
+type TableDialogVm = { form: { rows: number; cols: number; border: boolean; align: TableAlign } }
 
 // QEditor renders its toolbar buttons only after a deferred (setTimeout-based) refresh, so let a
 // real macrotask elapse before reading the toolbar, then settle the resulting re-render.
@@ -439,18 +441,39 @@ test("the table dialog inserts the shape it collected and rejects one past the b
 
     // A count over the limit has to be refused here: buildTableHtml clamps silently, so letting it
     // through gives the user a table that quietly isn't the one they asked for.
-    dialog.cols = MAX_TABLE_COLS + 1
+    dialog.form.cols = MAX_TABLE_COLS + 1
     await nextTick()
     await submitDialog(wrapper)
     expect(editor.vm.runCmd).not.toHaveBeenCalled()
 
-    dialog.rows = 2
-    dialog.cols = 2
+    dialog.form.rows = 2
+    dialog.form.cols = 2
+    dialog.form.border = false
+    dialog.form.align = "center"
     await nextTick()
     await submitDialog(wrapper)
     expect(editor.vm.runCmd).toHaveBeenCalledWith(
         "insertHTML",
-        "<table><thead><tr><th>&nbsp;</th><th>&nbsp;</th></tr></thead>" +
+        '<table border="0" align="center"><thead><tr><th>&nbsp;</th><th>&nbsp;</th></tr></thead>' +
             "<tbody><tr><td>&nbsp;</td><td>&nbsp;</td></tr></tbody></table><p><br></p>",
     )
 })
+
+test("closing the table dialog clears every field, not just the ones with a named default", async () => {
+    const wrapper = await mountEditor({ toolbar: FULL_TOOLBAR })
+    await openViaHandler(wrapper, "table")
+    const dialog = wrapper.findComponent(EditorTableDialog)
+    const vm = dialog.vm as unknown as TableDialogVm
+
+    vm.form.rows = 2
+    vm.form.border = false
+    vm.form.align = "center"
+    await nextTick()
+
+    // Without a full reset the next table silently inherits the last one's borders and alignment,
+    // which is invisible in the dialog because those controls look the same either way.
+    dialog.findComponent(RecordFormDialog).vm.$emit("hide")
+    await nextTick()
+
+    expect({ ...vm.form }).toStrictEqual({ rows: 3, cols: 3, header: true, border: true, align: "" })
+})

VueApp/src/components/editor/EditorTableDialog.vue

@@ -8,11 +8,11 @@
         form-error=""
         submit-label="Insert"
         @update:model-value="emit('update:modelValue', $event)"
-        @submit="emit('submit', { rows, cols, header })"
+        @submit="emit('submit', { ...form })"
         @hide="reset"
     >
         <q-input
-            v-model.number="rows"
+            v-model.number="form.rows"
             data-autofocus
             outlined
             dense
@@ -25,7 +25,7 @@
         />
 
         <q-input
-            v-model.number="cols"
+            v-model.number="form.cols"
             outlined
             dense
             type="number"
@@ -38,40 +38,61 @@
         />
 
         <q-checkbox
-            v-model="header"
+            v-model="form.header"
             label="First row is a header"
         />
+
+        <q-checkbox
+            v-model="form.border"
+            label="Show borders"
+        />
+
+        <q-select
+            v-model="form.align"
+            outlined
+            dense
+            options-dense
+            emit-value
+            map-options
+            label="Alignment"
+            :options="ALIGN_OPTIONS"
+        />
     </RecordFormDialog>
 </template>
 
 <script setup lang="ts">
-import { ref } from "vue"
+import { reactive } from "vue"
 import RecordFormDialog from "@/components/RecordFormDialog.vue"
+import type { TableAlign, TableOptions } from "@/components/editor/editor-html"
 import { MAX_TABLE_COLS, MAX_TABLE_ROWS } from "@/components/editor/editor-html"
 
-/** Collects the shape of a new table; the parent builds and inserts the HTML. */
+/** Collects the shape of a new table, in the order VIPER 1's CKEditor dialog asked for it; the parent
+ *  builds and inserts the HTML. */
 
 defineProps<{ modelValue: boolean }>()
 
 const emit = defineEmits<{
     "update:modelValue": [value: boolean]
-    submit: [value: { rows: number; cols: number; header: boolean }]
+    submit: [value: TableOptions]
 }>()
 
-const DEFAULT_ROWS = 3
-const DEFAULT_COLS = 3
+const ALIGN_OPTIONS: { label: string; value: TableAlign }[] = [
+    { label: "Not set", value: "" },
+    { label: "Left", value: "left" },
+    { label: "Center", value: "center" },
+    { label: "Right", value: "right" },
+]
+
+// One literal so a new field can't be added to the form and forgotten in reset().
+const DEFAULTS = { rows: 3, cols: 3, header: true, border: true, align: "" as TableAlign }
 
-const rows = ref(DEFAULT_ROWS)
-const cols = ref(DEFAULT_COLS)
-const header = ref(true)
+const form = reactive({ ...DEFAULTS })
 
 function inRange(value: number, max: number) {
     return Number.isInteger(value) && value >= 1 && value <= max
 }
 
 function reset() {
-    rows.value = DEFAULT_ROWS
-    cols.value = DEFAULT_COLS
-    header.value = true
+    Object.assign(form, DEFAULTS)
 }
 </script>

VueApp/src/components/editor/editor-html.ts

@@ -120,22 +120,34 @@ function tbody(rowCount: number, bodyRow: string): string {
 const MAX_TABLE_ROWS = 50
 const MAX_TABLE_COLS = 20
 
+/** Table alignment, written as the presentational align attribute; "" leaves the attribute off. */
+type TableAlign = "" | "left" | "center" | "right"
+
+interface TableOptions {
+    rows: number
+    cols: number
+    header: boolean
+    /** Off writes border="0", CKEditor's (VIPER 1) marker for a layout table, which base.css leaves unstyled. */
+    border?: boolean
+    align?: TableAlign
+}
+
 /**
  * Build a <table> skeleton. `rows` is the total row count including the header row when `header`
  * is true. Every cell holds &nbsp; so the caret can enter it in contenteditable. A trailing
  * `<p><br></p>` is appended so the user can type below the table.
  */
-function buildTableHtml(opts: { rows: number; cols: number; header: boolean }): string {
+function buildTableHtml(opts: TableOptions): string {
     const rows = clamp(opts.rows, 1, MAX_TABLE_ROWS)
     const cols = clamp(opts.cols, 1, MAX_TABLE_COLS)
 
     const headerRow = `<tr>${"<th>&nbsp;</th>".repeat(cols)}</tr>`
     const bodyRow = `<tr>${"<td>&nbsp;</td>".repeat(cols)}</tr>`
 
-    const table = opts.header
-        ? `<table><thead>${headerRow}</thead>${tbody(rows - 1, bodyRow)}</table>`
-        : `<table>${tbody(rows, bodyRow)}</table>`
-    return `${table}<p><br></p>`
+    const border = (opts.border ?? true) ? 1 : 0
+    const align = opts.align ? ` align="${opts.align}"` : ""
+    const inner = opts.header ? `<thead>${headerRow}</thead>${tbody(rows - 1, bodyRow)}` : tbody(rows, bodyRow)
+    return `<table border="${border}"${align}>${inner}</table><p><br></p>`
 }
 
 export {
@@ -150,4 +162,4 @@ export {
     MAX_TABLE_ROWS,
     MAX_TABLE_COLS,
 }
-export type { LinkKind }
+export type { LinkKind, TableAlign, TableOptions }

VueApp/src/styles/base.css

@@ -238,7 +238,7 @@ div.breadcrumbs {
 .q-table__bottom,
 .q-table__middle table thead tr:first-child th {
     /* bg color is important for th; just specify one */
-    background-color: #eee;
+    background-color: var(--table-header);
     white-space: nowrap;
 }
 
@@ -621,33 +621,33 @@ header .q-avatar {
     line-height: 1.4;
 }
 
-/* Tables and images inside sanitized CMS content, in the live block and the editor alike. Tables
-   carry no default borders, so a table inserted from the editor would be invisible without these.
-   A table too wide for the block scrolls inside it (overflow-x on the two containers) instead of
-   widening the page. */
+/* Tables and images inside sanitized CMS content, in the live block and the editor alike. A table
+   too wide for the block scrolls inside it (overflow-x on the two containers) instead of widening
+   the page. */
 .content-block,
 .content-block-editor .q-editor__content {
     overflow-x: auto;
 }
 
-.content-block table,
-.content-block-editor .q-editor__content table {
+:is(.content-block, .content-block-editor .q-editor__content) table {
     max-width: 100%;
+}
+
+/* Only tables that opt in with a border attribute get borders, cell padding and a header fill. The
+   editor's table dialog emits border="1"; legacy VIPER 1 content used the same attribute, with
+   border="0" meaning "no borders". Tables without it (mostly layout tables in migrated content)
+   keep the browser defaults they rendered with in VIPER 1. */
+:is(.content-block, .content-block-editor .q-editor__content) table[border]:not([border="0"]) {
     border-collapse: collapse;
 }
 
-.content-block th,
-.content-block td,
-.content-block-editor .q-editor__content th,
-.content-block-editor .q-editor__content td {
+:is(.content-block, .content-block-editor .q-editor__content) table[border]:not([border="0"]) :is(th, td) {
     padding: 0.25rem 0.5rem;
     border: 1px solid var(--ucdavis-black-20);
 }
 
-.content-block th,
-.content-block-editor .q-editor__content th {
-    /* The table-header token, matching the q-table header fill above */
-    background-color: #eee;
+:is(.content-block, .content-block-editor .q-editor__content) table[border]:not([border="0"]) th {
+    background-color: var(--table-header);
 }
 
 .content-block img,

VueApp/src/styles/colors.css

@@ -65,6 +65,10 @@
        ring in base.css / site.css so components can reference a token instead
        of repeating the hex. */
     --focus-ring-color: #258cfb;
+
+    /* Table header fill, shared by q-table headers and bordered CMS content
+       tables so the two treatments cannot drift apart. */
+    --table-header: #eee;
 }
 
 /* Background utility classes — UC Davis palette */

test/Services/HtmlSanitizerServiceTests.cs

@@ -140,6 +140,10 @@ public void Strips_legacy_custom_tags(string input)
     [InlineData("<a target=\"_blank\" rel=\"noopener\" href=\"https://example.com\">x</a>", "target=\"_blank\"", "rel=\"noopener\"")]
     [InlineData("<a href=\"https://example.com/file.pdf\" download=\"file.pdf\">x</a>", "download=\"file.pdf\"", "href=\"https://example.com/file.pdf\"")]
     [InlineData("<table><thead><tr><th scope=\"col\">h</th></tr></thead></table>", "<thead>", "scope=\"col\"")]
+    [InlineData("<table border=\"1\"><tbody><tr><td>x</td></tr></tbody></table>", "border=\"1\"", "<td>")]
+    [InlineData("<table align=\"center\"><tbody><tr><td align=\"right\">x</td></tr></tbody></table>", "align=\"center\"", "align=\"right\"")]
+    [InlineData("<table cellpadding=\"4\" cellspacing=\"0\"><tbody><tr><td>x</td></tr></tbody></table>", "cellpadding=\"4\"", "cellspacing=\"0\"")]
+    [InlineData("<table><tbody><tr valign=\"top\"><td>x</td></tr></tbody></table>", "valign=\"top\"", "<td>")]
     public void Preserves_allowed_constructs(string input, string mustContain1, string mustContain2)
     {
         var output = _sanitizer.Sanitize(input);

web/Services/HtmlSanitizerService.cs

@@ -51,6 +51,9 @@ private static HtmlSanitizer BuildSanitizer(bool allowDiffMarkers)
             {
                 "href", "src", "alt", "title", "class", "id", "name",
                 "width", "height", "colspan", "rowspan", "scope",
+                // Presentational table attributes migrated VIPER 1 content carries (the set legacy
+                // antisamy-cms.xml allowed); border additionally selects bordered styling in base.css.
+                "border", "align", "valign", "cellpadding", "cellspacing",
                 "target", "rel", "download",
                 "style"
             })