Closed cd-rite closed 2 years ago
Fine suggestion. I don't see why we should not update it. It is indeed a submodule and it checks out an old version of the plugin.
I wonder if we should update it now (and forget about again for another 7 years) or if we should make the submodule track HEAD of lightbox2. There are currently no tests which verify that the produced html (with lightbox2 javascript and styles) actually works, i.e. that the images popup when clicked...
I wonder if we should update it now (and forget about again for another 7 years) or if we should make the submodule track HEAD of lightbox2
I tend to use the former (use a fixed version) in that way you know when a problem suddenly surfaces that it can not be some lightbox update.
Yes, that is the advantage of a fixed version. The advantage of using HEAD of a suitable branch is it will be updated whenever there is a new release. But, as new releases are infrequent we can not count on those keeping lightbox2 up to date anyway.
I will update it to a new fixed version for now.
Yeah, it's a tricky issue. Do you have Dependabot alerts set up for this repo? At least then you would be alerted to an issue found in a fixed version, even if you are not tracking HEAD. I have the sphinx build product committed in our repo so we can serve out our docs internally, and that's what triggered Dependabot. I'm not sure if including a submodule the way you are would even trigger it....!
I wonder if we could, or should, use the jQuery which is distributed with sphinx. Currently Sphinx distribute jQuery 3.5.1.
Create a new sphinx project, build the HTML and you'll see _build/html/_static/jquery-3.5.1.js
.
phinx-quickstart --project "Sphinx jQuery" --author "J. Query" --quiet
make html
It appears easiest to keep it the way it currently is; sphinx-contrib images distribute its own jquery, but it feels a bit redundant ...
├── _build
│ ├── html
│ ├── genindex.html
│ ├── index.html
│ ├── objects.inv
│ ├── search.html
│ ├── searchindex.js
│ ├── _sources
│ │ └── index.rst.txt
│ └── _static
│ ├── jquery-3.5.1.js
│ ├── jquery.js
│ ├── sphinxcontrib-images
│ │ └── LightBox2
│ │ ├── lightbox2
│ │ │ ├── css
│ │ │ │ └── lightbox.css
│ │ │ ├── img
│ │ │ │ ├── close.png
│ │ │ │ ├── loading.gif
│ │ │ │ ├── next.png
│ │ │ │ └── prev.png
│ │ │ └── js
│ │ │ ├── jquery-1.11.0.min.js
│ │ │ ├── lightbox.min.js
│ │ │ └── lightbox.min.map
│ │ └── lightbox2-customize
│ │ └── jquery-noconflict.js
│ ├── underscore-1.13.1.js
│ └── underscore.js
My tests (July 2021) with sphinx-quickstart --project "Sphinx jQuery" --author "J. Query" --quiet make html
show this extension can indeed use the jQuery already included by sphinx.
<script data-url_root="./" id="documentation_options" src="_static/documentation_options.js"></script>
<script src="_static/jquery.js"></script>
<script src="_static/underscore.js"></script>
<script src="_static/doctools.js"></script>
<script src="_static/sphinxcontrib-images/LightBox2/lightbox2/js/jquery-1.11.0.min.js"></script>
<script src="_static/sphinxcontrib-images/LightBox2/lightbox2/js/lightbox.min.js"></script>
<script src="_static/sphinxcontrib-images/LightBox2/lightbox2-customize/jquery-noconflict.js"></script>
can be reduced to the following and lightbox2 still works:
<script data-url_root="./" id="documentation_options" src="_static/documentation_options.js"></script>
<script src="_static/jquery.js"></script>
<script src="_static/underscore.js"></script>
<script src="_static/doctools.js"></script>
<script src="_static/sphinxcontrib-images/LightBox2/lightbox2/js/lightbox.min.js"></script>
However, there seems to be a proposal to remove jQuery as a dependency for Sphinx: https://github.com/sphinx-doc/sphinx/issues/7405 . So I think it seem safer to ship jQuery with sphinxcontrib-images...
@cd-rite The jquery-1.11.0.min.js which is included with sphinxcontrib-images at present comes from lightbox2/js/jquery-1.11.0.min.js
(lightbox2@a534955) but in more recent versions of lightbox2 that file no longer exist.
Now lightbox2 ships with a lightbox2/dist/js/lightbox-plus-jquery.min.js
(lightbox2@f90a3b0 a file which combines the lightbox2 javascript and jquery in one file).
Would dependabot still work for you if we started shipping lightbox-plus-jquery.min.js
with sphinxcontrib-images?
@jonascj I think that should work! Thanks!
0.9.4 released with the updated version of Lightbox2 and hence jQuery.
It seems like the included version of LightBox2 is pointed at a specific, very old commit, which includes an old version of jquery. The newer versions of LightBox2 use later versions of jquery.
Actually, It looks like it's a git submodule that was committed 7 years ago and has not been updated since....