Skip to content

Commit

Permalink
Prevent duplicates in dfns extracts (#1110)
Browse files Browse the repository at this point in the history
This makes the dfns extraction code skip over a re-defined term (same linking
text, same type, same namespace), and report a warning when a duplicate dfn is
encountered and skipped.

Warnings are easily glossed over but we don't really have a better mechanism in
place right now to report issues found during extractions. Apart from making the
whole extraction fail that is, which seems overkill.

Fixes w3c/webref#783
  • Loading branch information
tidoust committed Nov 9, 2022
1 parent 92e8994 commit 2065cef
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
24 changes: 23 additions & 1 deletion src/browserlib/extract-dfns.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ import {parse} from "../../node_modules/webidl2/index.js";
* "prose" (last one indicates that definition appears in the main body of
* the spec)
*
* The extraction ignores definitions with an unknown type. A warning is issued
* to the console when that happens.
*
* The extraction uses the first definition it finds when it bumps into a term
* that is defined more than once (same "linkingText", same "type", same "for").
* A warning is issued to the console when that happens.
*
* @function
* @public
* @return {Array(Object)} An Array of definitions
Expand Down Expand Up @@ -99,6 +106,20 @@ function hasValidType(el) {
return isValid;
}

// Return true when definition is not already defined in the list,
// Return false and issue a warning when it is already defined.
function isNotAlreadyDefined(dfn, idx, list) {
const first = list.find(d => d === dfn ||
(d.type === dfn.type &&
d.linkingText.length === dfn.linkingText.length &&
d.linkingText.every(lt => dfn.linkingText.find(t => t == lt)) &&
d.for.length === dfn.for.length &&
d.for.every(lt => dfn.for.find(t => t === lt))));
if (first !== dfn) {
console.warn('[reffy]', `Duplicate dfn found for "${dfn.linkingText[0]}", type="${dfn.type}", for="${dfn.for[0]}"`);
}
return first === dfn;
}

function definitionMapper(el, idToHeading) {
let definedIn = 'prose';
Expand Down Expand Up @@ -237,7 +258,8 @@ export default function (spec, idToHeading = {}) {
const link = node.querySelector('a[href^="http"]');
return !link || (node.textContent.trim() !== link.textContent.trim());
})
.map(node => definitionMapper(node, idToHeading));
.map(node => definitionMapper(node, idToHeading))
.filter(isNotAlreadyDefined);
}

function preProcessEcmascript() {
Expand Down
4 changes: 4 additions & 0 deletions tests/extract-dfns.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,10 @@ const tests = [
html: "<dfn id=foo data-dfn-type=invalidtype>Foo</dfn>",
changesToBaseDfn: []
},
{title: "ignores dfns already defined",
html: "<dfn id='foo'>Foo</dfn>. <dfn id='foo2'>Foo</dfn> is already defined.",
changesToBaseDfn: [{}]
},
{title: "automatically fixes internal slots dfns with an invalid 'idl' data-dfn-type",
html: "<dfn id=foo data-dfn-type=idl>Foo</dfn>",
changesToBaseDfn: [{type: "attribute", access: "public"}]
Expand Down

0 comments on commit 2065cef

Please sign in to comment.