Skip to content

Consider removing private cssSelector logic in HTMLResourceContentIterator #188

@JayPanoz

Description

@JayPanoz

When tackling #117 in #185 I came across this logic for removing attributes such as xml:lang, epub:type, etc. because CSS Selector Generator is using querySelectorAll.

Turns out that was down do an old customised version of CSS Selector Generator (3.6.4) that did not handle namespaced attributes. It’s been fixed in 3.6.7 though, and we now have a script to handle it automatically in Navigator.

The scope of the chore PR being limited, there was not enough time available to attempt removing this at the time.

/**
* The css-selector-generator library chokes when generating a selector and using element
* attributes that have a namespace prefix, for example `xml:lang="fr"`. It's also not very
* useful to use these elements to generate a CSS selector that's usable with the querySelector
* DOM API, because that API can't use attributes with prefixes, unless you just select for any prefix,
* which, in certain edge cases, could result in the incorrect element being selected. So, in order to
* to solve the breakage in the library and avoid using these attributes in the first place, we just
* remove any attributes with namespace prefixes. There's still enough "meat" left to properly
* generate a CSS selector that can reliably be used with querySelector. Afterwards, we add them back
* to the element, so we can, for example, still select the xml:lang for accessibility purposes.
*/
const removedAttributes = new Map<HTMLElement, Attr[]>();
let currEl: HTMLElement | null = el;
while(currEl && currEl !== this.doc.body) {
for (let i = 0; i < currEl.attributes.length; i++) {
const attr = currEl.attributes[i];
if(attr.prefix) {
if(!removedAttributes.has(currEl)) {
removedAttributes.set(currEl, [attr])
} else {
removedAttributes.set(currEl, removedAttributes.get(currEl)!.concat([attr]))
}
currEl.attributes.removeNamedItem(attr.name);
}
}
currEl = currEl.parentElement;
}
const sel = getCssSelector(el, {
root: this.doc
});
removedAttributes.forEach((value, key) => value.forEach(v => key.attributes.setNamedItem(v)));
this.recipeCssSelectorCache.set(el, sel);
return sel;
}

Since pointerUp is no longer throwing an error when encountering epub:type for instance, we may be confident this workaround can be removed as well.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions