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]

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]

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]

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]

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 😄