Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash: opds publicationInfoA11y Component crash with strictNullChecks issue #2537

Closed
panaC opened this issue Sep 11, 2024 · 9 comments
Closed
Assignees

Comments

@panaC panaC self-assigned this Sep 11, 2024
@panaC panaC closed this as completed in e8db025 Oct 3, 2024
panaC added a commit that referenced this issue Oct 4, 2024
@panaC panaC reopened this Oct 24, 2024
@danielweck
Copy link
Member

the OPDS link above does not trigger the bug because the accessModeSufficient was removed.

@panaC
Copy link
Member Author

panaC commented Oct 24, 2024

the OPDS link above does not trigger the bug because the accessModeSufficient was removed.

https://raw.githack.com/edrlab/publications/85f16df059d19d1bfad254b1dbd8990a29f39ce2/feeds/thorium3_documentation.json

@danielweck
Copy link
Member

this link points to an invalid OPDS feed (not conforming to the JSON Schema) where accessModeSufficient is not an array.
(in this case, the crash isn't in Thorium, it is in the low-level parser which doesn't tolerate non-array property value)

@danielweck
Copy link
Member

see: edrlab/publications@c4f2019

@danielweck
Copy link
Member

danielweck commented Oct 24, 2024

accessModeSufficient is an array of string arrays ... but the string arrays containing a single item can be reduced to a string value.

JSON Schema:
https://github.com/readium/webpub-manifest/blob/03d7681cf1ff689bad76efaabc9c77423296a94c/schema/a11y.schema.json#L54-L82

@panaC
Copy link
Member Author

panaC commented Oct 28, 2024

7eae5aa

@panaC
Copy link
Member Author

panaC commented Oct 28, 2024

It was fixed with this commit : 7eae5aa

Works with https://rawcdn.githack.com/edrlab/thorium-reader-epub-tests/main/EPUB_Feeds_for_testing_purpose/opds-rare-epub-suit.json

I'm closing the issue, feel free to reopen it @gautierchomel
Thanks

@panaC panaC closed this as completed Oct 28, 2024
@danielweck
Copy link
Member

can we please test with a real world example like this:

accessModeSufficient: [ ["textual", "visual"], "textual" ]

and

accessModeSufficient: [ ["textual", "visual"], ["textual"] ]

http://kb.daisy.org/publishing/docs/metadata/schema.org/accessModeSufficient.html

thank you :)

(ps: instead of hand-crafting an OPDS feed for each case, maybe just force the values in Thorium post-parsing, pre-JSX?)

@panaC
Copy link
Member Author

panaC commented Oct 28, 2024

can we please test with a real world example like this:

accessModeSufficient: [ ["textual", "visual"], "textual" ]

and

accessModeSufficient: [ ["textual", "visual"], ["textual"] ]

http://kb.daisy.org/publishing/docs/metadata/schema.org/accessModeSufficient.html

thank you :)

(ps: instead of hand-crafting an OPDS feed for each case, maybe just force the values in Thorium post-parsing, pre-JSX?)

const findStrInArrayArray = (array: string[][] | string[] | undefined, str: string): boolean => Array.isArray(array) && array.findIndex((a) => (Array.isArray(a) ? a : [a]).findIndex((b) => b === str) > -1) > -1;

check on type : undefined | string[] | (string | string[])[]

Seems good, the issue throws by Gautier seems to be a misleading between the type string and string[].

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

No branches or pull requests

2 participants