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

Verify that jQuery selectors are escaped everywhere needed (epub-cfi.js?) #47

Open
danielweck opened this issue Jul 13, 2014 · 1 comment

Comments

@danielweck
Copy link
Member

This problem of XHTML / XML unique identifiers containing "special" characters was brought to my attention via the IDPF Readium forum (escaping rules for jQuery selectors):

http://idpf.org/forum/topic-1438

...so I resolved the issue for Media Overlays, and had a go at other areas as well:

26c7713

I used this regular expression to search the entire Readium codebase for potential occurrences of selectors that should be escaped (let me know if you think I may have missed some jQuery calls):

$(.+,.+)

Note that I have also checked every calls to Reader.getElement(spineItem, selector), which is a sneaky level of indirection to cfi_navigation_logic's own $(selector, this.getRootElement()) ... remember, we cannot escape indiscriminately, this has to be addressed in an ad-hoc manner, depending on what kind of selector is used (which is why we cannot simply escape the "selector" string variable).

I also double-checked CFI expressions that reference a single element with a UID assertion (e.g. ".../2/4[UID]"), and I did not see any problems. I used my Media Overlays CFI experiment (talking book with word-level highlighting), and I was able to work with an XHTML id such as "this.is.a.UID".

There is however one file that I did not want to touch, namely "epub_cfi.js" (primarily because it is not supposed to have access to ReadiumSDK.Helper functions, as it is an external library). For example:

retrieveItemRefHref
$("#" + $itemRefElement.attr("idref"), $packageDocument)

https://github.com/readium/readium-shared-js/blob/develop/lib/epub_cfi.js#L1669

As you can see, if idref contains forbidden characters, the selector matching will fail.

In the same file, I am pretty sure that these need escaping as well (albeit taking into consideration that they are attribute value selectors, not # UID):

generatePackageDocumentCFIComponent
$("itemref[idref='" + contentDocumentName + "']", $(packageDocument))

validatePackageDocument
$("itemref[idref='" + contentDocumentName + "']", packageDocument)

Email discussion:

https://groups.google.com/forum/#!topic/readium-dev/yDMmvT4cqCU

Bugzilla:
https://app.devzing.com/Readium/bugzilla/show_bug.cgi?id=47

@danielweck danielweck added this to the v1+ milestone Jul 31, 2014
@danielweck
Copy link
Member Author

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

No branches or pull requests

3 participants