fix(core/cddl): use conforming prose dfn attributes#5305
Open
aqilaziz wants to merge 2 commits into
Open
Conversation
Contributor
|
@aqilaziz, would you mind removing the build/ directory changes? |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes #5303 by removing the non-conformant authoring shortcuts (cddl-type/cddl-key/cddl-value attributes and for= on <dfn>) from core/cddl and switching to the standard data-dfn-type / data-dfn-for attributes. The plugin now discovers prose-level CDDL dfns by their conforming attributes and maps the generated CDDL block IDs to the prose dfn IDs so emitted links target existing prose definitions without creating duplicate block dfns.
Changes:
- Replace shorthand-attribute normalization in
normalizeProseDfnswith a query fordfn[data-dfn-type='cddl-…']and store generated→actual ID mapping (Set→Map). - Update the three
ReSpecCDDLMarkerresolution paths (type, key, value) to look up the prose dfn ID from the map and emithref="#${proseId}". - Update CDDL spec tests to author conforming dfns, and rebuild
builds/respec-w3c.js.
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/core/cddl.js | Stop accepting non-conformant shortcut attributes; map generated CDDL IDs to actual prose dfn IDs. |
| tests/spec/core/cddl-spec.js | Update prose-dfn tests to use data-dfn-type/data-dfn-for and add a dfn presence assertion. |
| builds/respec-w3c.js | Rebuilt artifact; contains the CDDL changes plus unrelated CRLF reflow noise in inline worker strings. |
Files not reviewed (1)
- builds/respec-w3c.js: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <h3 id="${s}">${e}</h3> | ||
| <div class="inside">${t}</div> | ||
| </div>`;const i=new Map([["labelledby",s]]);Ws(Gs,i),document.body.append(Vs,Gs),Vs.addEventListener("click",(()=>this.closeModal(n))),Vs.classList.toggle("respec-show-overlay"),Gs.hidden=!1,Qs(Gs)}};function ri(e){if("string"==typeof e)return e;const t=e.plugin?`<p class="respec-plugin">(plugin: "${e.plugin}")</p>`:"",n=e.hint?`\n${js(`<p class="respec-hint"><strong>How to fix:</strong> ${us(e.hint)}`,{inline:!e.hint.includes("\n")})}\n`:"",r=Array.isArray(e.elements)?`<p class="respec-occurrences">Occurred <strong>${e.elements.length}</strong> times at:</p>\n ${js(e.elements.map(si).join("\n"))}`:"",s=e.details?`\n\n<details>\n${e.details}\n</details>\n`:"";return`${js(`**${Pr(e.message)}**`,{inline:!0})}${n}${r}${s}${t}`}function si(e){return`* [\`<${e.localName}>\`](#${e.id}) element`}async function ii(e){try{ni.show(),await async function(){"loading"===document.readyState&&await new Promise((e=>document.addEventListener("DOMContentLoaded",e)))}(),await As(e)}finally{ni.enable()}}document.addEventListener("keydown",(e=>{"Escape"===e.key&&ni.closeModal()})),window.respecUI=ni,hs("error",(e=>ni.error(e))),hs("warn",(e=>ni.warning(e))),window.addEventListener("error",(e=>{console.error(e.error,e.message,e)}));const oi=[Promise.resolve().then((function(){return ci})),Promise.resolve().then((function(){return s})),Promise.resolve().then((function(){return pi})),Promise.resolve().then((function(){return io})),Promise.resolve().then((function(){return uo})),Promise.resolve().then((function(){return go})),Promise.resolve().then((function(){return $o})),Promise.resolve().then((function(){return Lo})),Promise.resolve().then((function(){return qs})),Promise.resolve().then((function(){return Ao})),Promise.resolve().then((function(){return Io})),Promise.resolve().then((function(){return jo})),Promise.resolve().then((function(){return Qi})),Promise.resolve().then((function(){return Uo})),Promise.resolve().then((function(){return qo})),Promise.resolve().then((function(){return Bo})),Promise.resolve().then((function(){return Go})),Promise.resolve().then((function(){return ic})),Promise.resolve().then((function(){return cc})),Promise.resolve().then((function(){return yc})),Promise.resolve().then((function(){return wc})),Promise.resolve().then((function(){return xc})),Promise.resolve().then((function(){return Rc})),Promise.resolve().then((function(){return Nc})),Promise.resolve().then((function(){return Oc})),Promise.resolve().then((function(){return zc})),Promise.resolve().then((function(){return cl})),Promise.resolve().then((function(){return gl})),Promise.resolve().then((function(){return $a})),Promise.resolve().then((function(){return Ol})),Promise.resolve().then((function(){return Ql})),Promise.resolve().then((function(){return _l})),Promise.resolve().then((function(){return cu})),Promise.resolve().then((function(){return Ia})),Promise.resolve().then((function(){return ku})),Promise.resolve().then((function(){return xu})),Promise.resolve().then((function(){return _o})),Promise.resolve().then((function(){return Cu})),Promise.resolve().then((function(){return Su})),Promise.resolve().then((function(){return Tu})),Promise.resolve().then((function(){return Du})),Promise.resolve().then((function(){return ju})),Promise.resolve().then((function(){return Mu})),Promise.resolve().then((function(){return Hu})),Promise.resolve().then((function(){return ld})),Promise.resolve().then((function(){return yd})),Promise.resolve().then((function(){return $d})),Promise.resolve().then((function(){return _d})),Promise.resolve().then((function(){return Ld})),Promise.resolve().then((function(){return Od})),Promise.resolve().then((function(){return jd})),Promise.resolve().then((function(){return Fd})),Promise.resolve().then((function(){return Jd})),Promise.resolve().then((function(){return Kc})),Promise.resolve().then((function(){return tp})),Promise.resolve().then((function(){return op})),Promise.resolve().then((function(){return cp})),Promise.resolve().then((function(){return up})),Promise.resolve().then((function(){return yp})),Promise.resolve().then((function(){return kp})),Promise.resolve().then((function(){return _p})),Promise.resolve().then((function(){return Tp})),Promise.resolve().then((function(){return Rp})),Promise.resolve().then((function(){return Lp})),Promise.resolve().then((function(){return Np})),Promise.resolve().then((function(){return zp})),Promise.resolve().then((function(){return qp})),Promise.resolve().then((function(){return Hp})),Promise.resolve().then((function(){return Jp})),Promise.resolve().then((function(){return Qp})),Promise.resolve().then((function(){return nh})),Promise.resolve().then((function(){return ih})),Promise.resolve().then((function(){return uh})),Promise.resolve().then((function(){return gh})),Promise.resolve().then((function(){return wh})),Promise.resolve().then((function(){return $h})),Promise.resolve().then((function(){return _h})),Promise.resolve().then((function(){return Lh}))];Promise.all(oi).then((e=>ii(e))).catch((e=>console.error(e)));var ai=Object.freeze({__proto__:null,default:'// ReSpec Worker\n"use strict";\n// hljs is either inlined by core/worker.js (preferred) or loaded below via\n// importScripts as a fallback when the inline fetch was not possible.\nif (typeof self.hljs === "undefined" && self.RESPEC_HIGHLIGHT_URL) {\n try {\n importScripts(self.RESPEC_HIGHLIGHT_URL);\n } catch (err) {\n console.error("Network error loading highlighter", err);\n }\n}\n\nself.addEventListener("message", ({ data }) => {\n switch (data.action) {\n case "highlight-load-lang": {\n const { langURL, langScript, propName, lang } = data;\n console.warn(\n `[ReSpec] The "highlight-load-lang" worker action is deprecated ` +\n `and will be removed in a future version. ` +\n `To migrate, fetch your language script in the main thread and ` +\n `send the text as "langScript" instead of "langURL". ` +\n `The "langURL" path may fail in Firefox. ` +\n `See https://github.com/speced/respec/issues/5228`\n );\n try {\n if (langScript) {\n const blob = new Blob([langScript], {\n type: "application/javascript",\n });\n const objectURL = URL.createObjectURL(blob);\n try {\n importScripts(objectURL);\n } finally {\n URL.revokeObjectURL(objectURL);\n }\n } else if (langURL) {\n const { protocol, hostname } = new URL(langURL);\n const isSecure =\n protocol === "https:" ||\n (protocol === "http:" &&\n (hostname === "localhost" ||\n hostname === "127.0.0.1" ||\n hostname === "[::1]"));\n if (!isSecure) {\n throw new Error(\n `langURL must be https: or http: on localhost, got "${langURL}"`\n );\n }\n importScripts(langURL);\n } else {\n throw new Error(\n `No langScript or langURL provided for language "${lang}"`\n );\n }\n if (typeof self[propName] === "function") {\n self.hljs.registerLanguage(lang, self[propName]);\n } else {\n throw new Error(\n `Language definer "${propName}" is not a function on self`\n );\n }\n } catch (err) {\n console.error("Failed to load or register language", lang, err);\n }\n delete data.langScript;\n delete data.langURL;\n break;\n }\n case "highlight": {\n const { code } = data;\n const langs = data.languages?.length ? data.languages : undefined;\n try {\n const { value, language } = self.hljs.highlightAuto(code, langs);\n Object.assign(data, { value, language });\n } catch (err) {\n console.error("Could not transform some code?", err);\n Object.assign(data, { value: code, language: "" });\n }\n break;\n }\n }\n self.postMessage(data);\n});\n'});var ci=Object.freeze({__proto__:null,name:"core/location-hash",run:function(){window.location.hash&&document.respec.ready.then((()=>{let e=decodeURIComponent(window.location.hash).slice(1);const t=document.getElementById(e),n=/\W/.test(e);if(!t&&n){const t=e.replace(/[\W]+/gim,"-").replace(/^-+/,"").replace(/-+$/,"");document.getElementById(t)&&(e=t)}window.location.hash=`#${e}`}))}});const li="w3c/group",ui="https://respec.org/w3c/groups/";async function di(e){let t="",n=e;e.includes("/")&&([t,n]=e.split("/",2));const r=new URL(`${n}/${t}`,ui),s=await zr(r.href);if(s.ok){const e=await s.json(),{id:t,name:n,patentURI:r,patentPolicy:i,type:o,wgURI:a}=e;return{wg:n,wgId:t,wgURI:a,wgPatentURI:r,wgPatentPolicy:i,groupType:o}}const i=await s.text();let o,a=`Failed to fetch group details (HTTP: ${s.status}).`;409===s.status?[a,o]=i.split("\n",2):404===s.status&&(o=ls`See the list of [supported group names](https://respec.org/w3c/groups/) to use with the ${"[group]"} configuration option.`),ns(a,li,{hint:o})}var pi=Object.freeze({__proto__:null,name:li,run:async function(e){if(!e.group)return;const{group:t}=e,n=Array.isArray(t)?await async function(e){const t=await Promise.all(e.map(di)),n={wg:[],wgId:[],wgURI:[],wgPatentURI:[],wgPatentPolicy:[],groupType:[]};for(const e of t.filter(Boolean))for(const t of Object.keys(n))n[t].push(e[t]);return n}(t):await di(t);Object.assign(e,n)}});function hi(e){if(!e.key){const t="Found a link without `key` attribute in the configuration. See dev console.";return rs(t,"core/templates/show-link"),void console.warn(t,e)}return sr` | ||
| </div>`;const i=new Map([["labelledby",s]]);Ws(Gs,i),document.body.append(Vs,Gs),Vs.addEventListener("click",(()=>this.closeModal(n))),Vs.classList.toggle("respec-show-overlay"),Gs.hidden=!1,Qs(Gs)}};function ri(e){if("string"==typeof e)return e;const t=e.plugin?`<p class="respec-plugin">(plugin: "${e.plugin}")</p>`:"",n=e.hint?`\n${js(`<p class="respec-hint"><strong>How to fix:</strong> ${us(e.hint)}`,{inline:!e.hint.includes("\n")})}\n`:"",r=Array.isArray(e.elements)?`<p class="respec-occurrences">Occurred <strong>${e.elements.length}</strong> times at:</p>\n ${js(e.elements.map(si).join("\n"))}`:"",s=e.details?`\n\n<details>\n${e.details}\n</details>\n`:"";return`${js(`**${Pr(e.message)}**`,{inline:!0})}${n}${r}${s}${t}`}function si(e){return`* [\`<${e.localName}>\`](#${e.id}) element`}async function ii(e){try{ni.show(),await async function(){"loading"===document.readyState&&await new Promise((e=>document.addEventListener("DOMContentLoaded",e)))}(),await As(e)}finally{ni.enable()}}document.addEventListener("keydown",(e=>{"Escape"===e.key&&ni.closeModal()})),window.respecUI=ni,hs("error",(e=>ni.error(e))),hs("warn",(e=>ni.warning(e))),window.addEventListener("error",(e=>{console.error(e.error,e.message,e)}));const oi=[Promise.resolve().then((function(){return ci})),Promise.resolve().then((function(){return s})),Promise.resolve().then((function(){return pi})),Promise.resolve().then((function(){return io})),Promise.resolve().then((function(){return uo})),Promise.resolve().then((function(){return go})),Promise.resolve().then((function(){return $o})),Promise.resolve().then((function(){return Lo})),Promise.resolve().then((function(){return qs})),Promise.resolve().then((function(){return Ao})),Promise.resolve().then((function(){return Io})),Promise.resolve().then((function(){return jo})),Promise.resolve().then((function(){return Qi})),Promise.resolve().then((function(){return Uo})),Promise.resolve().then((function(){return qo})),Promise.resolve().then((function(){return Bo})),Promise.resolve().then((function(){return Go})),Promise.resolve().then((function(){return ic})),Promise.resolve().then((function(){return cc})),Promise.resolve().then((function(){return yc})),Promise.resolve().then((function(){return wc})),Promise.resolve().then((function(){return xc})),Promise.resolve().then((function(){return Rc})),Promise.resolve().then((function(){return Nc})),Promise.resolve().then((function(){return Oc})),Promise.resolve().then((function(){return zc})),Promise.resolve().then((function(){return cl})),Promise.resolve().then((function(){return gl})),Promise.resolve().then((function(){return $a})),Promise.resolve().then((function(){return Ol})),Promise.resolve().then((function(){return Ql})),Promise.resolve().then((function(){return _l})),Promise.resolve().then((function(){return cu})),Promise.resolve().then((function(){return Ia})),Promise.resolve().then((function(){return ku})),Promise.resolve().then((function(){return xu})),Promise.resolve().then((function(){return _o})),Promise.resolve().then((function(){return Cu})),Promise.resolve().then((function(){return Su})),Promise.resolve().then((function(){return Tu})),Promise.resolve().then((function(){return Du})),Promise.resolve().then((function(){return ju})),Promise.resolve().then((function(){return Mu})),Promise.resolve().then((function(){return Hu})),Promise.resolve().then((function(){return ld})),Promise.resolve().then((function(){return yd})),Promise.resolve().then((function(){return $d})),Promise.resolve().then((function(){return _d})),Promise.resolve().then((function(){return Ld})),Promise.resolve().then((function(){return Od})),Promise.resolve().then((function(){return jd})),Promise.resolve().then((function(){return Fd})),Promise.resolve().then((function(){return Jd})),Promise.resolve().then((function(){return Kc})),Promise.resolve().then((function(){return tp})),Promise.resolve().then((function(){return op})),Promise.resolve().then((function(){return cp})),Promise.resolve().then((function(){return up})),Promise.resolve().then((function(){return yp})),Promise.resolve().then((function(){return kp})),Promise.resolve().then((function(){return _p})),Promise.resolve().then((function(){return Tp})),Promise.resolve().then((function(){return Rp})),Promise.resolve().then((function(){return Lp})),Promise.resolve().then((function(){return Np})),Promise.resolve().then((function(){return zp})),Promise.resolve().then((function(){return qp})),Promise.resolve().then((function(){return Hp})),Promise.resolve().then((function(){return Jp})),Promise.resolve().then((function(){return Qp})),Promise.resolve().then((function(){return nh})),Promise.resolve().then((function(){return ih})),Promise.resolve().then((function(){return uh})),Promise.resolve().then((function(){return gh})),Promise.resolve().then((function(){return wh})),Promise.resolve().then((function(){return $h})),Promise.resolve().then((function(){return _h})),Promise.resolve().then((function(){return Lh}))];Promise.all(oi).then((e=>ii(e))).catch((e=>console.error(e)));var ai=Object.freeze({__proto__:null,default:'// ReSpec Worker\r\n"use strict";\r\n// hljs is either inlined by core/worker.js (preferred) or loaded below via\r\n// importScripts as a fallback when the inline fetch was not possible.\r\nif (typeof self.hljs === "undefined" && self.RESPEC_HIGHLIGHT_URL) {\r\n try {\r\n importScripts(self.RESPEC_HIGHLIGHT_URL);\r\n } catch (err) {\r\n console.error("Network error loading highlighter", err);\r\n }\r\n}\r\n\r\nself.addEventListener("message", ({ data }) => {\r\n switch (data.action) {\r\n case "highlight-load-lang": {\r\n const { langURL, langScript, propName, lang } = data;\r\n console.warn(\r\n `[ReSpec] The "highlight-load-lang" worker action is deprecated ` +\r\n `and will be removed in a future version. ` +\r\n `To migrate, fetch your language script in the main thread and ` +\r\n `send the text as "langScript" instead of "langURL". ` +\r\n `The "langURL" path may fail in Firefox. ` +\r\n `See https://github.com/speced/respec/issues/5228`\r\n );\r\n try {\r\n if (langScript) {\r\n const blob = new Blob([langScript], {\r\n type: "application/javascript",\r\n });\r\n const objectURL = URL.createObjectURL(blob);\r\n try {\r\n importScripts(objectURL);\r\n } finally {\r\n URL.revokeObjectURL(objectURL);\r\n }\r\n } else if (langURL) {\r\n const { protocol, hostname } = new URL(langURL);\r\n const isSecure =\r\n protocol === "https:" ||\r\n (protocol === "http:" &&\r\n (hostname === "localhost" ||\r\n hostname === "127.0.0.1" ||\r\n hostname === "[::1]"));\r\n if (!isSecure) {\r\n throw new Error(\r\n `langURL must be https: or http: on localhost, got "${langURL}"`\r\n );\r\n }\r\n importScripts(langURL);\r\n } else {\r\n throw new Error(\r\n `No langScript or langURL provided for language "${lang}"`\r\n );\r\n }\r\n if (typeof self[propName] === "function") {\r\n self.hljs.registerLanguage(lang, self[propName]);\r\n } else {\r\n throw new Error(\r\n `Language definer "${propName}" is not a function on self`\r\n );\r\n }\r\n } catch (err) {\r\n console.error("Failed to load or register language", lang, err);\r\n }\r\n delete data.langScript;\r\n delete data.langURL;\r\n break;\r\n }\r\n case "highlight": {\r\n const { code } = data;\r\n const langs = data.languages?.length ? data.languages : undefined;\r\n try {\r\n const { value, language } = self.hljs.highlightAuto(code, langs);\r\n Object.assign(data, { value, language });\r\n } catch (err) {\r\n console.error("Could not transform some code?", err);\r\n Object.assign(data, { value: code, language: "" });\r\n }\r\n break;\r\n }\r\n }\r\n self.postMessage(data);\r\n});\r\n'});var ci=Object.freeze({__proto__:null,name:"core/location-hash",run:function(){window.location.hash&&document.respec.ready.then((()=>{let e=decodeURIComponent(window.location.hash).slice(1);const t=document.getElementById(e),n=/\W/.test(e);if(!t&&n){const t=e.replace(/[\W]+/gim,"-").replace(/^-+/,"").replace(/-+$/,"");document.getElementById(t)&&(e=t)}window.location.hash=`#${e}`}))}});const li="w3c/group",ui="https://respec.org/w3c/groups/";async function di(e){let t="",n=e;e.includes("/")&&([t,n]=e.split("/",2));const r=new URL(`${n}/${t}`,ui),s=await zr(r.href);if(s.ok){const e=await s.json(),{id:t,name:n,patentURI:r,patentPolicy:i,type:o,wgURI:a}=e;return{wg:n,wgId:t,wgURI:a,wgPatentURI:r,wgPatentPolicy:i,groupType:o}}const i=await s.text();let o,a=`Failed to fetch group details (HTTP: ${s.status}).`;409===s.status?[a,o]=i.split("\n",2):404===s.status&&(o=ls`See the list of [supported group names](https://respec.org/w3c/groups/) to use with the ${"[group]"} configuration option.`),ns(a,li,{hint:o})}var pi=Object.freeze({__proto__:null,name:li,run:async function(e){if(!e.group)return;const{group:t}=e,n=Array.isArray(t)?await async function(e){const t=await Promise.all(e.map(di)),n={wg:[],wgId:[],wgURI:[],wgPatentURI:[],wgPatentPolicy:[],groupType:[]};for(const e of t.filter(Boolean))for(const t of Object.keys(n))n[t].push(e[t]);return n}(t):await di(t);Object.assign(e,n)}});function hi(e){if(!e.key){const t="Found a link without `key` attribute in the configuration. See dev console.";return rs(t,"core/templates/show-link"),void console.warn(t,e)}return sr` |
aqilaziz
force-pushed
the
fix/5303-cddl-conformant-prose-dfns
branch
from
9dfce0f to
fc1415a
Compare
Author
|
Removed the The PR diff now only includes:
Verification:
I did not rerun the full Node lint locally because dependencies are not installed in this checkout and this machine is currently low on disk space. |
Author
|
Updated this PR to address the failing checks:
Verified locally:
|
Contributor
|
@aqilaziz builds/ are back 😊… also, have the agent take into context the original issue. The problem is not in the duplicate definitions (duplicates are an authoring error). Definitions should only appear once per document, not that the once in prose are preferred. The bug is that we accidentally allowed non-conforming HTML attributes in the original design (e.g, “for=“). Does that make sense? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5303.
Summary
Tests