Open hedsnz opened 1 year ago
Thanks a lot, a PR would be highly appreciated :bow:
I've looked into slimbox2.js a bit further, and it hasn't been updated since 2015. It probably isn't compatible with jQuery 3, either. And if I inspect the browser console after this MRE,
library(pander)
myReport <- Pandoc$new()
myReport$format <- "html"
myReport$add(plot(1:10))
myReport$export()
I'm getting several JS uncaught syntax errors, including from slimbox2.js and jquery-1.7.2.min.js.
So I guess there are a couple of questions. Is slimbox working/doing anything in the current version? Is there a MRE showing it working? And if not, can it just be removed entirely?
If that's the case then I'll submit a PR with slimbox removed and jQuery updated to 3.x. It would also be good to understand what other components of pander rely on jQuery, though, for testing/update purposes.
Thanks
Thank you very much for raising this, @hedsnz!
I think this might indeed be a bit larger task after all, as the CSS template and JS functions have not been updated for years ... and the CDN that used to support the pander
package at cdn.rapporter.net has also been gone in the past years.
Although I can revive that latter if needed, but a proper review would make much more sense now, as the whole HTML report structure was created 10 years (!) ago.
Anyway, putting aside the scope creep problem and focusing on your original question: I feel OK about dropping slimbox2.js
from the project, and update jQuery. Once slimbox is gone, the only jQuery-based stuff remaining is the menu builder as per https://github.com/Rapporter/pander/blob/master/inst/includes/javascripts/custom.js -- probably not affected by the update.
The following XSS and prototype pollution vulnerabilities are present in the legacy version of jQuery included in pander (v1.7.2):
It appears that
jquery.min.js
is required forslimbox2.js
which is called incustom.js
. All three are included ininst/includes/html/header.html
.Is it possible to update jQuery to 3.6.x? I'm happy to submit a PR for this, let me know.