From 82a3cf3f239f3234dff0928821d6cb4e6d671c14 Mon Sep 17 00:00:00 2001 From: Dominique Hazael-Massieux Date: Thu, 21 Dec 2023 15:29:53 +0100 Subject: [PATCH 1/9] Add data-defines on well-known patterns of term/definition association As discussed in #4522 See also https://github.com/w3c/reffy/pull/1444 --- src/core/dfn.js | 22 ++++++++++++++++++++++ tests/spec/core/dfn-spec.js | 28 ++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/src/core/dfn.js b/src/core/dfn.js index 96d3c7e19e..c6047e4d92 100644 --- a/src/core/dfn.js +++ b/src/core/dfn.js @@ -237,4 +237,26 @@ function addContractDefaults() { for (const dfn of exportableDfns) { dfn.dataset.export = ""; } + + // - Sets data-defines on well-known definition content patterns + + // A dt with class dfn-desc (on in a dl with such a class) + // containing a definition + // indicates that the following dd or div element contains its prose content + for (const dt of document.querySelectorAll("dl.dfn-desc dt:has(dfn[data-dfn-type]), dt.dfn-desc:has(dfn[data-dfn-type])")) { + const dfnId = dt.querySelector("dfn[id]").id; + const dfnContent = dt.nextElementSibling; + if (dfnId && ["DIV", "DD"].includes(dfnContent.tagName)) { + dfnContent.dataset.defines = `#${dfnId}`; + } + } + + // a non-dt element with class dfn-desc containing a definition + // indicates that the said element contains its prose content + for (const el of document.querySelectorAll(":not(dt):not(dl).dfn-desc:has(dfn[data-dfn-type])")) { + const dfnId = el.querySelector("dfn[id]").id; + if (dfnId) { + el.dataset.defines = `#${dfnId}`; + } + } } diff --git a/tests/spec/core/dfn-spec.js b/tests/spec/core/dfn-spec.js index fb1b4003e7..ecee316651 100644 --- a/tests/spec/core/dfn-spec.js +++ b/tests/spec/core/dfn-spec.js @@ -655,5 +655,33 @@ describe("Core — Definitions", () => { expect(errors[0].message).toContain("Declares both"); expect(errors[1].message).toContain("but also has a"); }); + + it("assigns data-defines on well-known pattern", async () => { + const body = ` +
+

Definition and its description

+

+ A definition can also have an associated description +

+
+
different convention
+
Different conventions can be applied to associate a term with its description
+
+
+
local convention
+
The local convention can be applied to a dt individually
+
+
+ `; + const ops = makeStandardOps(null, body); + const doc = await makeRSDoc(ops); + const desc1 = doc.getElementById("desc1"); + const desc2 = doc.getElementById("desc2"); + const desc3 = doc.getElementById("desc3"); + expect(desc1.dataset.defines).toBe("#dfn-definition"); + expect(desc2.dataset.defines).toBe("#dfn-different-convention"); + expect(desc3.dataset.defines).toBe("#dfn-local-convention"); + }); + }); }); From 5150f017366ae82892971a8e5fe30d9c4aab8126 Mon Sep 17 00:00:00 2001 From: Dominique Hazael-Massieux Date: Thu, 21 Dec 2023 15:45:59 +0100 Subject: [PATCH 2/9] Fix lint --- src/core/dfn.js | 19 +++++++++++++++---- tests/spec/core/dfn-spec.js | 1 - 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/core/dfn.js b/src/core/dfn.js index c6047e4d92..2af51e7c54 100644 --- a/src/core/dfn.js +++ b/src/core/dfn.js @@ -243,17 +243,28 @@ function addContractDefaults() { // A dt with class dfn-desc (on in a dl with such a class) // containing a definition // indicates that the following dd or div element contains its prose content - for (const dt of document.querySelectorAll("dl.dfn-desc dt:has(dfn[data-dfn-type]), dt.dfn-desc:has(dfn[data-dfn-type])")) { + /** @type NodeListOf */ + const describedDTs = document.querySelectorAll( + "dl.dfn-desc dt:has(dfn[data-dfn-type]), dt.dfn-desc:has(dfn[data-dfn-type])" + ); + for (const dt of describedDTs) { const dfnId = dt.querySelector("dfn[id]").id; - const dfnContent = dt.nextElementSibling; - if (dfnId && ["DIV", "DD"].includes(dfnContent.tagName)) { + + const dfnContent = /** @type {HTMLElement | null} */ ( + dt.nextElementSibling + ); + if (dfnContent && dfnId && ["DIV", "DD"].includes(dfnContent.tagName)) { dfnContent.dataset.defines = `#${dfnId}`; } } // a non-dt element with class dfn-desc containing a definition // indicates that the said element contains its prose content - for (const el of document.querySelectorAll(":not(dt):not(dl).dfn-desc:has(dfn[data-dfn-type])")) { + /** @type NodeListOf */ + const otherDescriptionContainers = document.querySelectorAll( + ":not(dt):not(dl).dfn-desc:has(dfn[data-dfn-type])" + ); + for (const el of otherDescriptionContainers) { const dfnId = el.querySelector("dfn[id]").id; if (dfnId) { el.dataset.defines = `#${dfnId}`; diff --git a/tests/spec/core/dfn-spec.js b/tests/spec/core/dfn-spec.js index ecee316651..a27d759038 100644 --- a/tests/spec/core/dfn-spec.js +++ b/tests/spec/core/dfn-spec.js @@ -682,6 +682,5 @@ describe("Core — Definitions", () => { expect(desc2.dataset.defines).toBe("#dfn-different-convention"); expect(desc3.dataset.defines).toBe("#dfn-local-convention"); }); - }); }); From 4204017b88e5111f921089c99a22738e4d65851f Mon Sep 17 00:00:00 2001 From: Dominique Hazael-Massieux Date: Thu, 21 Dec 2023 16:46:48 +0100 Subject: [PATCH 3/9] Fix typo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: François Daoust --- src/core/dfn.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/dfn.js b/src/core/dfn.js index 2af51e7c54..17fcba39ad 100644 --- a/src/core/dfn.js +++ b/src/core/dfn.js @@ -240,7 +240,7 @@ function addContractDefaults() { // - Sets data-defines on well-known definition content patterns - // A dt with class dfn-desc (on in a dl with such a class) + // A dt with class dfn-desc (or in a dl with such a class) // containing a definition // indicates that the following dd or div element contains its prose content /** @type NodeListOf */ From 2ca26dd65523ef6963d4d57e654a202aff2fe5e9 Mon Sep 17 00:00:00 2001 From: Dominique Hazael-Massieux Date: Thu, 21 Dec 2023 16:50:01 +0100 Subject: [PATCH 4/9] Use data-dfn-type consistently as a dfn selector --- src/core/dfn.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/dfn.js b/src/core/dfn.js index 17fcba39ad..95cea2c219 100644 --- a/src/core/dfn.js +++ b/src/core/dfn.js @@ -248,7 +248,7 @@ function addContractDefaults() { "dl.dfn-desc dt:has(dfn[data-dfn-type]), dt.dfn-desc:has(dfn[data-dfn-type])" ); for (const dt of describedDTs) { - const dfnId = dt.querySelector("dfn[id]").id; + const dfnId = dt.querySelector("dfn[data-dfn-type]").id; const dfnContent = /** @type {HTMLElement | null} */ ( dt.nextElementSibling @@ -265,7 +265,7 @@ function addContractDefaults() { ":not(dt):not(dl).dfn-desc:has(dfn[data-dfn-type])" ); for (const el of otherDescriptionContainers) { - const dfnId = el.querySelector("dfn[id]").id; + const dfnId = el.querySelector("dfn[data-dfn-type]").id; if (dfnId) { el.dataset.defines = `#${dfnId}`; } From 6a8fe49cb9d1c1cd5fb5e004d60771f62943db8f Mon Sep 17 00:00:00 2001 From: Dominique Hazael-Massieux Date: Fri, 22 Dec 2023 08:43:40 +0100 Subject: [PATCH 5/9] Address review feedback https://github.com/w3c/respec/pull/4620#discussion_r1434706130 and https://github.com/w3c/respec/pull/4620#issuecomment-1867208920 --- src/core/dfn.js | 20 +++++++++++++------- tests/spec/core/dfn-spec.js | 6 +++--- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/core/dfn.js b/src/core/dfn.js index 95cea2c219..493021ca62 100644 --- a/src/core/dfn.js +++ b/src/core/dfn.js @@ -78,7 +78,7 @@ export function run() { } dfn.dataset.lt = titles.join("|"); } - sub("plugins-done", addContractDefaults); + sub("plugins-done", completeDefinitionMarkup); } /** @@ -216,6 +216,11 @@ function processAsInternalSlot(title, dfn) { return dfnType; } +function completeDefinitionMarkup() { + addContractDefaults(); + addDefinitionPointers(); +} + function addContractDefaults() { // Find all dfns that don't have a type and default them to "dfn". /** @type NodeListOf */ @@ -237,15 +242,16 @@ function addContractDefaults() { for (const dfn of exportableDfns) { dfn.dataset.export = ""; } +} - // - Sets data-defines on well-known definition content patterns - - // A dt with class dfn-desc (or in a dl with such a class) +// - Sets data-defines on well-known definition content patterns +function addDefinitionPointers() { + // A dt with class definition (or in a dl with such a class) // containing a definition // indicates that the following dd or div element contains its prose content /** @type NodeListOf */ const describedDTs = document.querySelectorAll( - "dl.dfn-desc dt:has(dfn[data-dfn-type]), dt.dfn-desc:has(dfn[data-dfn-type])" + "dl.definition dt:has(dfn[data-dfn-type]), dt.definition:has(dfn[data-dfn-type])" ); for (const dt of describedDTs) { const dfnId = dt.querySelector("dfn[data-dfn-type]").id; @@ -258,11 +264,11 @@ function addContractDefaults() { } } - // a non-dt element with class dfn-desc containing a definition + // a non-dt element with class definition containing a definition // indicates that the said element contains its prose content /** @type NodeListOf */ const otherDescriptionContainers = document.querySelectorAll( - ":not(dt):not(dl).dfn-desc:has(dfn[data-dfn-type])" + ":not(dt):not(dl).definition:has(dfn[data-dfn-type])" ); for (const el of otherDescriptionContainers) { const dfnId = el.querySelector("dfn[data-dfn-type]").id; diff --git a/tests/spec/core/dfn-spec.js b/tests/spec/core/dfn-spec.js index a27d759038..b482609043 100644 --- a/tests/spec/core/dfn-spec.js +++ b/tests/spec/core/dfn-spec.js @@ -660,15 +660,15 @@ describe("Core — Definitions", () => { const body = `

Definition and its description

-

+

A definition can also have an associated description

-
+
different convention
Different conventions can be applied to associate a term with its description
-
local convention
+
local convention
The local convention can be applied to a dt individually
From 246b81a0b0838b3cdd93da5ec06d5a9232b69414 Mon Sep 17 00:00:00 2001 From: Dominique Hazael-Massieux Date: Fri, 22 Dec 2023 17:02:39 +0100 Subject: [PATCH 6/9] Use more intuitive pattern to associate descriptions to their terms As suggested in https://github.com/w3c/respec/pull/4620#discussion_r1435000012 --- src/core/dfn.js | 39 ++++++++++++++++++------------------- tests/spec/core/dfn-spec.js | 13 +++++-------- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/src/core/dfn.js b/src/core/dfn.js index 493021ca62..88fb8b813a 100644 --- a/src/core/dfn.js +++ b/src/core/dfn.js @@ -246,34 +246,33 @@ function addContractDefaults() { // - Sets data-defines on well-known definition content patterns function addDefinitionPointers() { - // A dt with class definition (or in a dl with such a class) - // containing a definition - // indicates that the following dd or div element contains its prose content + // A dl with class hasdefinitions marks all next siblings of dt with the class + // definition /** @type NodeListOf */ const describedDTs = document.querySelectorAll( - "dl.definition dt:has(dfn[data-dfn-type]), dt.definition:has(dfn[data-dfn-type])" + "dl.hasdefinitions dt" ); for (const dt of describedDTs) { - const dfnId = dt.querySelector("dfn[data-dfn-type]").id; - - const dfnContent = /** @type {HTMLElement | null} */ ( - dt.nextElementSibling - ); - if (dfnContent && dfnId && ["DIV", "DD"].includes(dfnContent.tagName)) { - dfnContent.dataset.defines = `#${dfnId}`; - } + dt.nextElementSibling.classList.add("definition"); } - // a non-dt element with class definition containing a definition - // indicates that the said element contains its prose content + // an element with class "definition" is marked as defining the term + // found either it the element itself, or in the closest previous sibling with + // a defined term /** @type NodeListOf */ - const otherDescriptionContainers = document.querySelectorAll( - ":not(dt):not(dl).definition:has(dfn[data-dfn-type])" + const definitionContainers = document.querySelectorAll( + ".definition" ); - for (const el of otherDescriptionContainers) { - const dfnId = el.querySelector("dfn[data-dfn-type]").id; - if (dfnId) { - el.dataset.defines = `#${dfnId}`; + for (const el of definitionContainers) { + let curEl = el; + let dfn; + while (curEl) { + dfn = curEl.querySelector("dfn[data-dfn-type]"); + if (dfn) break; + curEl = curEl.previousElementSibling; + } + if (dfn?.id) { + el.dataset.defines = `#${dfn.id}`; } } } diff --git a/tests/spec/core/dfn-spec.js b/tests/spec/core/dfn-spec.js index b482609043..c7b2b8f2fc 100644 --- a/tests/spec/core/dfn-spec.js +++ b/tests/spec/core/dfn-spec.js @@ -656,21 +656,18 @@ describe("Core — Definitions", () => { expect(errors[1].message).toContain("but also has a"); }); - it("assigns data-defines on well-known pattern", async () => { + it("assigns data-defines on well-known patterns", async () => { const body = `
-

Definition and its description

+

Definition and its description

+

A description explains what a term is.

A definition can also have an associated description

-
+
different convention
Different conventions can be applied to associate a term with its description
-
-
local convention
-
The local convention can be applied to a dt individually
-
`; const ops = makeStandardOps(null, body); @@ -680,7 +677,7 @@ describe("Core — Definitions", () => { const desc3 = doc.getElementById("desc3"); expect(desc1.dataset.defines).toBe("#dfn-definition"); expect(desc2.dataset.defines).toBe("#dfn-different-convention"); - expect(desc3.dataset.defines).toBe("#dfn-local-convention"); + expect(desc3.dataset.defines).toBe("#dfn-description"); }); }); }); From 075bba503b410f447446ad5d92a061e8686579ee Mon Sep 17 00:00:00 2001 From: Dominique Hazael-Massieux Date: Fri, 22 Dec 2023 17:08:39 +0100 Subject: [PATCH 7/9] Fix lint --- src/core/dfn.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/core/dfn.js b/src/core/dfn.js index 88fb8b813a..e19af47a0c 100644 --- a/src/core/dfn.js +++ b/src/core/dfn.js @@ -249,9 +249,7 @@ function addDefinitionPointers() { // A dl with class hasdefinitions marks all next siblings of dt with the class // definition /** @type NodeListOf */ - const describedDTs = document.querySelectorAll( - "dl.hasdefinitions dt" - ); + const describedDTs = document.querySelectorAll("dl.hasdefinitions dt"); for (const dt of describedDTs) { dt.nextElementSibling.classList.add("definition"); } @@ -260,16 +258,14 @@ function addDefinitionPointers() { // found either it the element itself, or in the closest previous sibling with // a defined term /** @type NodeListOf */ - const definitionContainers = document.querySelectorAll( - ".definition" - ); + const definitionContainers = document.querySelectorAll(".definition"); for (const el of definitionContainers) { let curEl = el; let dfn; while (curEl) { dfn = curEl.querySelector("dfn[data-dfn-type]"); if (dfn) break; - curEl = curEl.previousElementSibling; + curEl = /** @type {HTMLElement | null} */ (curEl.previousElementSibling); } if (dfn?.id) { el.dataset.defines = `#${dfn.id}`; From d2e169ad86b3fdbed2c4cdf35fe57128016a595b Mon Sep 17 00:00:00 2001 From: Dominique Hazael-Massieux Date: Mon, 8 Jan 2024 15:39:12 +0100 Subject: [PATCH 8/9] Revert to use only dl.hasdefinitions and in-element dfn detection Based on review comments https://github.com/w3c/respec/pull/4620#discussion_r1437398798 https://github.com/w3c/respec/pull/4620#discussion_r1437393883 --- src/core/dfn.js | 33 ++++++++++++++++++--------------- tests/spec/core/dfn-spec.js | 5 +---- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/core/dfn.js b/src/core/dfn.js index e19af47a0c..ab3ea5401a 100644 --- a/src/core/dfn.js +++ b/src/core/dfn.js @@ -246,28 +246,31 @@ function addContractDefaults() { // - Sets data-defines on well-known definition content patterns function addDefinitionPointers() { - // A dl with class hasdefinitions marks all next siblings of dt with the class - // definition + // A dl with class hasdefinitions associated the dfn in each dt + // the definition in the following sibling element /** @type NodeListOf */ - const describedDTs = document.querySelectorAll("dl.hasdefinitions dt"); + const describedDTs = document.querySelectorAll( + "dl.hasdefinitions dt:has(dfn[data-dfn-type])" + ); for (const dt of describedDTs) { - dt.nextElementSibling.classList.add("definition"); + const dfnId = dt.querySelector("dfn[data-dfn-type]").id; + const dfnContent = /** @type {HTMLElement | null} */ ( + dt.nextElementSibling + ); + if (dfnContent && !dfnContent.dataset.defines && dfnId) { + dfnContent.dataset.defines = `#${dfnId}`; + } } // an element with class "definition" is marked as defining the term - // found either it the element itself, or in the closest previous sibling with - // a defined term + // found in the element /** @type NodeListOf */ - const definitionContainers = document.querySelectorAll(".definition"); + const definitionContainers = document.querySelectorAll( + ".definition:has(dfn[data-dfn-type])" + ); for (const el of definitionContainers) { - let curEl = el; - let dfn; - while (curEl) { - dfn = curEl.querySelector("dfn[data-dfn-type]"); - if (dfn) break; - curEl = /** @type {HTMLElement | null} */ (curEl.previousElementSibling); - } - if (dfn?.id) { + const dfn = el.querySelector("dfn[data-dfn-type]"); + if (dfn.id && !el.dataset.defines) { el.dataset.defines = `#${dfn.id}`; } } diff --git a/tests/spec/core/dfn-spec.js b/tests/spec/core/dfn-spec.js index c7b2b8f2fc..af70a2f6ac 100644 --- a/tests/spec/core/dfn-spec.js +++ b/tests/spec/core/dfn-spec.js @@ -659,8 +659,7 @@ describe("Core — Definitions", () => { it("assigns data-defines on well-known patterns", async () => { const body = `
-

Definition and its description

-

A description explains what a term is.

+

Definitions

A definition can also have an associated description

@@ -674,10 +673,8 @@ describe("Core — Definitions", () => { const doc = await makeRSDoc(ops); const desc1 = doc.getElementById("desc1"); const desc2 = doc.getElementById("desc2"); - const desc3 = doc.getElementById("desc3"); expect(desc1.dataset.defines).toBe("#dfn-definition"); expect(desc2.dataset.defines).toBe("#dfn-different-convention"); - expect(desc3.dataset.defines).toBe("#dfn-description"); }); }); }); From a7698133cd78e1ebcfa67f7fe822fca93e4d0898 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20C=C3=A1ceres?= Date: Thu, 11 Jan 2024 10:44:56 +1100 Subject: [PATCH 9/9] Apply suggestions from code review --- src/core/dfn.js | 2 +- tests/spec/core/dfn-spec.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/dfn.js b/src/core/dfn.js index ab3ea5401a..aac0e65ca6 100644 --- a/src/core/dfn.js +++ b/src/core/dfn.js @@ -250,7 +250,7 @@ function addDefinitionPointers() { // the definition in the following sibling element /** @type NodeListOf */ const describedDTs = document.querySelectorAll( - "dl.hasdefinitions dt:has(dfn[data-dfn-type])" + "dl.definitions dt:has(dfn[data-dfn-type])" ); for (const dt of describedDTs) { const dfnId = dt.querySelector("dfn[data-dfn-type]").id; diff --git a/tests/spec/core/dfn-spec.js b/tests/spec/core/dfn-spec.js index af70a2f6ac..d9f4c438fc 100644 --- a/tests/spec/core/dfn-spec.js +++ b/tests/spec/core/dfn-spec.js @@ -663,7 +663,7 @@ describe("Core — Definitions", () => {

A definition can also have an associated description

-
+
different convention
Different conventions can be applied to associate a term with its description