Skip to content

Conversation

@JayPanoz
Copy link
Contributor

@JayPanoz JayPanoz commented Jan 19, 2026

This PR removes the vanilla test app and updates dependencies across packages.

This closes #117 through f10ef93 by scripting the modifications needed in CSS Selector Generator dependencies (Navigator > FrameBlobBuilder)

It adds all colon attributes e.g. epub:type in blacklist by default when outputting the custom script but maybe we want to scope this. I am not sure at the moment, despite testing the change – because it is tough to encounter edge cases from a limited amount of publications in Thorium Web.

We happen to inject CSSSelectorGenerator in the Blob with a slight modification of replacing cssSelectorGenerator with a readium-prefixed version. This is not necessarily maintainable as this is hardcoded, and needs to be updated cautiously every time the dependency is updated.
This also attemps to get rid of the error happening with `epub:type` attribute
@JayPanoz JayPanoz marked this pull request as ready for review January 20, 2026 14:54
@JayPanoz JayPanoz requested a review from chocolatkey January 20, 2026 14:54
Copy link
Member

@chocolatkey chocolatkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • RIP testapp
  • Eventually we should come up with a better solution for the css selector code, but should suffice for now

@JayPanoz
Copy link
Contributor Author

Yeah re. css selector, I came across this instance in services:

/**
* 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;
}

At first sight it would be tempting to align it on blacklist but admittedly testing the change is quite an issue. There has to be a reason it was done like this, and I’m uncomfortable updating it in this PR without the entire context – that would be the FXL Navigator PR so it’s massive in scope, and that type of details might go forgotten.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Uncaught SyntaxError: Failed to execute 'querySelectorAll' on 'Document': '[epub:type]' is not a valid selector.

3 participants