Photoswipe outdated, should be upgraded and made an optional feature

[scorpi11]

Describe the bug

The embedded Photoswipe copy is an outdated version from 2015:

/*! PhotoSwipe - v4.1.1 - 2015-12-24

However, the nature of the project changed a lot and Photoswipe is not a "simple jQuery based JS library" anymore but requires npm to build as of version 5.

An upgrade to the recent version is strongly recommended.

Furthermore, Photoswipe support should be made an optional feature which is not turned on by default, in both cases (sticking to the old version and upgrading to the new version). When sticking with the old version, users can be warned that they enable support for an outdated JS library. When upgrading to the new version, only users who want to use Photoswipe will require npm at build time. Another reason is that Linux distributions don't like embedded libraries for multiple reasons.

Steps to reproduce

Go to data/pswp in the source code, check version of Photoswipe.

Expected behavior

darktable should not ship with outdated JS libraries.

Logfile | Screenshot | Screencast

No response

Commit

No response

Where did you obtain darktable from?

downloaded from www.darktable.org

darktable version

4.6

What OS are you using?

Linux

What is the version of your OS?

Debian Sid

Describe your system?

No response

Are you using OpenCL GPU in darktable?

None

If yes, what is the GPU card and driver?

No response

Please provide additional context if applicable. You can attach files too, but might need to rename to .txt or .zip

No response

[kmilos]

FYI, upstream bug report: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=969159

[scorpi11]

I know, I came from there and started to check what can be done to fix it.

[scorpi11]

One further enhancement came into my mind: it should be possible to use a system wide Photoswipe installation (like it is possible with libraw), so distributions can remove the embedded Photoswipe copy from their packages.

However, there is currently no package for any distribution (I checked Fedora, Gentoo, Ubuntu, Debian).

[github-actions[bot]]

This issue has been marked as stale due to inactivity for the last 60 days. It will be automatically closed in 300 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.

[scorpi11]

This issue is still present in master. What to the devs think about updating to current Photoswipe?

[github-actions[bot]]

This issue has been marked as stale due to inactivity for the last 60 days. It will be automatically closed in 300 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.

[victoryforce]

However, the nature of the project changed a lot and Photoswipe is not a "simple jQuery based JS library" anymore but requires npm to build as of version 5.

@scorpi11 That is, we can use the result of this build by adding the resulting files when exporting to the "web gallery" target?

(looking at the project repo) Maybe we can just grab the files we need from dist/?

[victoryforce]

@scorpi11 Unfortunately, I have not received an answer to the question whether it is possible to simply replace the files with the files of the new version.

My thoughts on this issue are as follows:

• If the photoswipe code that darktable includes in the web pages it writes for export has a security issue, it should be fixed. I couldn't find any mention of security issues though.

• Since photoswipe is not packaged in distributions, as was mentioned, there is nothing to discuss about using system photoswipe instead of native code. It is simply impossible and it is not known when it will be possible.

• If the new version can be simply added by replacing the files of the current version, there is no problem to do so, regardless of whether there are any useful changes specifically for use in darktable. But this requires a person who knows photoswipe well.

• The first look at the latest version of photoswipe gives the impression that the code has evolved significantly, so trivial replacement of the same files with newer versions is impossible.

Considering all the above, I would say that I personally have no motivation to spend time to try to update photoswipe in the absence of a real need (ie, security problems in the used version). Volunteers are welcome though.

[github-actions[bot]]

This issue has been marked as stale due to inactivity for the last 60 days. It will be automatically closed in 300 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.

[wpferguson]

@TurboGit does this count as a bugfix (should I try and get it done before code freeze?) or should it go in 5.2?

EDIT: or should we rip it out and replace it with a Lua solution?

EDIT 2: I'm currently prototyping it in Lua so that I can distribute to those interested for testing and comments.

[TurboGit]

@wpferguson : Not a bug as nothing is broken :) It works even if outdated IIUC.

Not sure how one could replace it by Lua. The js for the gallery cannot be Lua? Can you clarify what you have in mind? TIA.

[scorpi11]

I guess the idea was to replace the C code in the export function, which builds the "dynamic part" of the gallery, by a Lua script.

[wpferguson]

There was a discussion in the darktable channel about this issue started by @victoryforce with @paperdigits, @elstoc, and @scorpi11 participating. Somewhere in the discussion the possibility of replacing the gallery with Lua was raised and before the "smart" part of my brain kicked in I kind of "volunteered".

In the discussion the prospect of using CSSBox as a replacement for photoswipe was proposed (discussed?).

If you generate a 5 image gallery and replace the index.html with the following and add the cssbox.css file to the style directory then open it in a browser you can see what it looks like and how it works.

index.html.txt cssbox.css.txt

Also discussed was the current styling of the gallery and how it could use some "help".

I've looked at the code (imageio/storage/gallery.c) and I can replace the generated html with cssbox compliant html, which solves the photoswipe problem. But, it doesn't fix the styling help problem. If I generate a PR to fix photoswipe, the only people that can test it are those that can build from master (and are willing to). This may not reach the part of the audience that can help with the styling.

But, I can build storages with Lua scripts also. So I can build the equivalent storage, then users can drop the script into their existing scripts directory and add the cssbox.css and style.css files to the lua/data directory. At that point they can build galleries and test, whether they are on master or not and hopefully generate a new style.css (or multiples so we have a choice).

For 5.2 I plan on adding the Lua scripts to the release which will remove the last barrier to entry for those who don't want to|can't install git. At this point the scripts become "built-in". I plan on adding a "system" directory for scripts that should always run because they provide some type of core feature such as group persistence across database instances. With this in place, the website gallery functionality could be a script. I don't have a preference, it's simply a possiblity.

[github-actions[bot]]

This issue has been marked as stale due to inactivity for the last 60 days. It will be automatically closed in 300 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.

[ptilopteri]

• github-actions[bot] @.***> [01-27-25 19:23]:

ping

-- (paka)Patrick Shanahan Plainfield, Indiana, USA @ptilopteri facebook/ptilopteri Photos: http://wahoo.no-ip.org/piwigo paka @ IRCnet oftc

[wpferguson]

Guess I should revisit it, since we're past release :smile:

[github-actions[bot]]

This issue has been marked as stale due to inactivity for the last 60 days. It will be automatically closed in 300 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.

[pabl0]

The current version of photoswipe seem to work perfectly fine, but I think there is some room for improvement in mobile support.

The photo counter/number in the left hand top corner and the buttons in the right hand corner are quite tiny when viewed in a mobile browser (at least Firefox/Android). They are much more prominent in the current version of photoswipe. Also, the actual thumbnail grid is also rather small in mobile and there is excess white space around it.

Perhaps it would not be very difficult to make the current version more mobile friendly with some minor CSS tweaks (and I guess by also adding viewport setting like "width=device-width,initial-scale=1".

[dimsemenov]

hey Darktable team,

PhotoSwipe author here, @pabl0 made me aware of this issue. Feel free to reach out to me if you need some help with integration.

Also, I am developing v6, with a target release in the next few months. The API changes from v5 to v6 will be minimal in contrast to v4. Perhaps you wanna wait for it.

[pabl0]

* Since photoswipe is not packaged in distributions, as was mentioned, there is nothing to discuss about using system photoswipe instead of native code. It is simply impossible and it is not known when it will be possible.

There is an Debian RFP since 2018 already, but it was blocked due to unclear licensing with v4, which is no longer a problem.

However, I am trying to imagine how using a system wide install of pswp would actually work with dt. Since the exported web gallery is often copied over to some other server, you really cannot expect the files to be available in some certain path in the hosting environment (likely running some totally different OS). The export tree is basically a standalone set of static files. Photoswipe is pretty lightweight, so disk space is not a major issue. Perhaps dt could use the system wide installation (if available at build time) as the source of files to be embedded in the export file tree, then at least pswp could be upgraded separately by the distro.

[scorpi11]

Perhaps dt could use the system wide installation (if available at build time) as the source of files to be embedded in the export file tree, then at least pswp could be upgraded separately by the distro.

This would still require npm in the darktable build procedure in case that a system wide PhotoSwipe is not available or should not be used for some other reason. What is the view of other developers regarding this?

@wpferguson already mentioned his thoughts to move the gallery generation to a Lua script.

Maybe a simple static gallery can be created with modern HTML/CSS/JS features without the need of external packages and npm and without significant maintenance effort. I already created a fullscreen slideshow that loads the images from a JSON file this way. I'll try to create the thumbnail gallery part to create a replacement for the embedded PhotoSwipe.

[wpferguson]

require npm

npm is in the headlines every day for another security exploit. The farther from that we can get the better IMO.

I also found a way to generate the gallery only using only CSS for the interaction. I'll dig it out and add it to this comment.

website_gallery.zip

[scorpi11]

I created a Lua script which implements an export module for a web gallery. It uses static files for HTML, CSS and JS.

The actual gallery data is loaded from a JSON file which is generated during export.

I'll try to implement all features from the old PhotoSwipe gallery. Currently, the zoom function is missing.

@wpferguson suggested to extend the Lua API to get an equivalent of dt_loc_get_datadir(). This is needed to find the static gallery files and copy it to the destination directory for the gallery. Once this is implemented, I'll create a PR.

[scorpi11]

@scorpi11 That is, we can use the result of this build by adding the resulting files when exporting to the "web gallery" target?

(looking at the project repo) Maybe we can just grab the files we need from dist/?

That would include generated files in the darktable source code. Doing so will open another issue. At least Debian doesn't like the usage of generated files in the source code.

[github-actions[bot]]

This issue has been marked as stale due to inactivity for the last 60 days. It will be automatically closed in 300 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.