Extract delegate for TrixEditorElement

seanpdoyle · seanpdoyle · commit bc24ac570d36 · 2024-02-08T23:44:43.000-05:00
In preparation for [#1128][], this commit introduces a module-private `Delegate` class to serve as a representation of what form integration requires for the `<trix-editor>` custom element. The structure of the `Delegate` class mirrors that of the `TrixEditorElement` from which its contents are extracted. First, there are the properties that mimic those of most form controls, including: * `labels` * `form` * `name` * `value` * `defaultValue` * `type` With the exception of `labels`, property access is mostly proxied through the associated `<input type="hidden">` element (accessed through its own `inputElement` property). Next, the `Delegate` defines methods that correspond to the Custom Element lifecycle events, including: * `connectedCallback` * `disconnectedCallback` * `setFormValue` The connected and disconnected callbacks mirror that of the `TrixEditorElement` itself. These callbacks attach and remove event listeners for `click` and `reset` events. The `setFormValue` is named to correspond with [ElementInternals.setFormValue][]. Along with introducing this callback method, this commit renames the `TrixEditorElement.setInputElementValue` method to `TrixEditorElement.setFormValue`. In addition to renaming `setInputElementValue`, this commit also defines `TrixEditorElement.formResetCallback` (along with other [empty form callbacks][]), then implements `TrixEditorElement.reset` as an alias. The name mirrors the [ElementInternals.formResetCallback][]. [#1128]: #1128 [ElementInternals.setFormValue]: https://developer.mozilla.org/en-US/docs/Web/API/ElementInternals/setFormValue [empty form callbacks]: https://webkit.org/blog/13711/elementinternals-and-form-associated-custom-elements/ [ElementInternals.formResetCallback]: https://web.dev/articles/more-capable-form-controls#void_formresetcallback
diff --git a/README.md b/README.md
@@ -79,6 +79,33 @@ To populate a `<trix-editor>` with stored content, include that content in the a
 
 Always use an associated input element to safely populate an editor. Trix won’t load any HTML content inside a `<trix-editor>…</trix-editor>` tag.
 
+## Providing an Accessible Name
+
+Like other form controls, `<trix-editor>` elements should have an accessible name. The `<trix-editor>` element integrates with `<label>` elements and The `<trix-editor>` supports two styles of integrating with `<label>` elements:
+
+1. render the `<trix-editor>` element with an `[id]` attribute that the `<label>` element references through its `[for]` attribute:
+
+```html
+<label for="editor">Editor</label>
+<trix-editor id="editor"></trix-editor>
+```
+
+2. render the `<trix-editor>` element as a child of the `<label>` element:
+
+```html
+<trix-toolbar id="editor-toolbar"></trix-toolbar>
+<label>
+  Editor
+
+  <trix-editor toolbar="editor-toolbar"></trix-editor>
+</label>
+```
+
+> [!WARNING]
+> When rendering the `<trix-editor>` element as a child of the `<label>` element, [explicitly render](#creating-an-editor) the corresponding `<trix-toolbar>` element outside of the `<label>` element.
+
+In addition to integrating with `<label>` elements, `<trix-editor>` elements support `[aria-label]` and `[aria-labelledby]` attributes.
+
 ## Styling Formatted Content
 
 To ensure what you see when you edit is what you see when you save, use a CSS class name to scope styles for Trix formatted content. Apply this class name to your `<trix-editor>` element, and to a containing element when you render stored Trix content for display in your application.
diff --git a/assets/index.html b/assets/index.html
@@ -72,7 +72,8 @@
   </head>
   <body>
     <main>
-      <trix-editor autofocus class="trix-content" input="input"></trix-editor>
+      <label for="editor">Input</label>
+      <trix-editor autofocus class="trix-content" input="input" id="editor"></trix-editor>
       <details id="output">
         <summary>Output</summary>
         <textarea readonly id="input"></textarea>
diff --git a/karma.conf.js b/karma.conf.js
@@ -3,7 +3,7 @@ const config = {
   frameworks: [ "qunit" ],
   files: [
     { pattern: "dist/test.js", watched: false },
-    { pattern: "src/test_helpers/fixtures/*.png", watched: false, included: false, served: true }
+    { pattern: "src/test/test_helpers/fixtures/*.png", watched: false, included: false, served: true }
   ],
   proxies: {
     "/test_helpers/fixtures/": "/base/src/test_helpers/fixtures/"
diff --git a/src/test/system/custom_element_test.js b/src/test/system/custom_element_test.js
@@ -442,7 +442,7 @@ testGroup("Custom element API", { template: "editor_empty" }, () => {
 
   test("editor resets to its original value on form reset", async () => {
     const element = getEditorElement()
-    const { form } = element.inputElement
+    const { form } = element
 
     await typeCharacters("hello")
     form.reset()
@@ -451,7 +451,7 @@ testGroup("Custom element API", { template: "editor_empty" }, () => {
 
   test("editor resets to last-set value on form reset", async () => {
     const element = getEditorElement()
-    const { form } = element.inputElement
+    const { form } = element
 
     element.value = "hi"
     await typeCharacters("hello")
@@ -461,7 +461,7 @@ testGroup("Custom element API", { template: "editor_empty" }, () => {
 
   test("editor respects preventDefault on form reset", async () => {
     const element = getEditorElement()
-    const { form } = element.inputElement
+    const { form } = element
     const preventDefault = (event) => event.preventDefault()
 
     await typeCharacters("hello")
@@ -471,12 +471,43 @@ testGroup("Custom element API", { template: "editor_empty" }, () => {
     form.removeEventListener("reset", preventDefault, false)
     expectDocument("hello\n")
   })
+
+  test("element returns empty string when value is missing", async () => {
+    const element = getEditorElement()
+
+    assert.equal(element.value, "")
+  })
+
+  test("editor resets to its original value on element reset", async () => {
+    const element = getEditorElement()
+
+    await typeCharacters("hello")
+    element.reset()
+    expectDocument("\n")
+  })
+
+  test("editor returns its type", async() => {
+    const element = getEditorElement()
+
+    assert.equal("trix-editor", element.type)
+  })
+})
+
+testGroup("HTML sanitization", { template: "editor_html" }, () => {
+  test("ignores text nodes in script elements", () => {
+    const element = getEditorElement()
+    element.value = "<div>safe</div><script>alert(\"unsafe\")</script>"
+
+    expectDocument("safe\n")
+    assert.equal(element.innerHTML, "<div><!--block-->safe</div>")
+    assert.equal(element.value, "<div>safe</div>")
+  })
 })
 
 testGroup("<label> support", { template: "editor_with_labels" }, () => {
   test("associates all label elements", () => {
     const labels = [ document.getElementById("label-1"), document.getElementById("label-3") ]
-    assert.deepEqual(getEditorElement().labels, labels)
+    assert.deepEqual(Array.from(getEditorElement().labels), labels)
   })
 
   test("focuses when <label> clicked", () => {
@@ -504,7 +535,7 @@ testGroup("form property references its <form>", { template: "editors_with_forms
     assert.equal(editor.form, form)
   })
 
-  test("transitively accesses its related <input> element's <form>", () => {
+  test("transitively accesses its related <form>", () => {
     const form = document.getElementById("input-form")
     const editor = document.getElementById("editor-with-input-form")
     assert.equal(editor.form, form)
diff --git a/src/test/system/installation_process_test.js b/src/test/system/installation_process_test.js
@@ -20,23 +20,27 @@ testGroup("Installation process", { template: "editor_html" }, () => {
   })
 })
 
-testGroup("Installation process without specified elements", { template: "editor_empty" }, () =>
-  test("creates identified toolbar and input elements", () => {
+testGroup("Installation process without specified elements", { template: "editor_empty" }, () => {
+  test("creates identified toolbar elements", () => {
     const editorElement = getEditorElement()
 
     const toolbarId = editorElement.getAttribute("toolbar")
     assert.ok(/trix-toolbar-\d+/.test(toolbarId), `toolbar id not assert.ok ${JSON.stringify(toolbarId)}`)
     const toolbarElement = document.getElementById(toolbarId)
     assert.ok(toolbarElement, "toolbar element not assert.ok")
     assert.equal(editorElement.toolbarElement, toolbarElement)
+  })
+
+  test("creates identified input elements", () => {
+    const editorElement = getEditorElement()
 
     const inputId = editorElement.getAttribute("input")
     assert.ok(/trix-input-\d+/.test(inputId), `input id not assert.ok ${JSON.stringify(inputId)}`)
     const inputElement = document.getElementById(inputId)
     assert.ok(inputElement, "input element not assert.ok")
     assert.equal(editorElement.inputElement, inputElement)
   })
-)
+})
 
 testGroup("Installation process with specified elements", { template: "editor_with_toolbar_and_input" }, () => {
   test("uses specified elements", () => {
diff --git a/src/trix/controllers/editor_controller.js b/src/trix/controllers/editor_controller.js
@@ -503,7 +503,7 @@ export default class EditorController extends Controller {
   updateInputElement() {
     const element = this.compositionController.getSerializableElement()
     const value = serializeToContentType(element, "text/html")
-    return this.editorElement.setInputElementValue(value)
+    return this.editorElement.setFormValue(value)
   }
 
   notifyEditorElement(message, data) {
diff --git a/src/trix/elements/trix_editor_element.js b/src/trix/elements/trix_editor_element.js

Original file line number	Diff line number	Diff line change
`@@ -503,7 +503,7 @@ export default class EditorController extends Controller {`
`503`	`503`	`updateInputElement() {`
`504`	`504`	`const element = this.compositionController.getSerializableElement()`
`505`	`505`	`const value = serializeToContentType(element, "text/html")`
`506`		`- return this.editorElement.setInputElementValue(value)`
	`506`	`+ return this.editorElement.setFormValue(value)`
`507`	`507`	`}`
`508`	`508`
`509`	`509`	`notifyEditorElement(message, data) {`