[5.x] Provide compatibility with "svg_image" and "svg_image_field" modules

Happy to review patches / MR's here. Feel free to set this back to active or needs review then.

I'm not sure if photoswipe 4 itself supports SVGs... that should be checked first.

Rating this as minor / edge-case. Let's test this with v5 later.

Here is a dirty (CSS) workaround to prevent (end)users from running into this. Otherwise the user will see an error message in the PSWP layer.

// Quickfix for the missing Photoswipe / PSWP SVG support
// Make the link not clickable.
// See: https://www.drupal.org/project/photoswipe/issues/3294118
a.photoswipe:has(img[src*=".svg"]) {
  pointer-events: none;
}

Alright that should be it. But TBH I am unsure if this currently makes sense, as the js photoswipe library doesn't provide a white background for svgs, So on many svgs you can't really see anything.

@anybody I guess we need to create a photoswipe js issue and fix it there?

The issue only stands out because you're testing with SVG files that, in all likelihood, will never be used in Photoswipe as-is. It's just as likely that a transparent background is what's needed.

In my opinion, this isn't a relevant issue.

Thanks @grevil great work. I added a little, but important, comment and think this is good to go. Regarding the background let's wait for user feedback and reports.

Good enough to be merged IMHO. Started merge train.

Ok two final things we need to do before we can publish this:
Provide a way to change the background for the photoswipe image view. To do so we need to conditionally set css like:

.pswp {
  --pswp-bg: #fff;
}

(e.g. for white background)
The background opacity is already configurable in the photoswipe global settings. I'd say we provide a background setting on global and formatter level (if set formatter level overrides global). As for "normal" photoswipe formatters we usually want to have the "normal" dark background, but for the svg formatter we want a white background. Making the setting setable on both levels gives us maximum flexibility.

Furthermore I am against putting the svg sanitizator in the main module composer dependency. The sanitization library already ships with svg_image, but not with svg_image_field. "svg_image_field" itself has some basic upload validation, but I would still like to sanitize the ouput with the same controller.

Maybe we should recommend the library in the composer.json and then add a warning if it isn't installed?

@anybody nono, stopped the merge train.

We should at least fix the composer dependency. Having a dependency on a library, the module usually doesn't need should be fixed here.

For the other point we can add a follow-up.

The background opacity is already configurable in the photoswipe global settings. I'd say we provide a background setting on global and formatter level (if set formatter level overrides global). As for "normal" photoswipe formatters we usually want to have the "normal" dark background, but for the svg formatter we want a white background. Making the setting setable on both levels gives us maximum flexibility.

I'd be interested to know what the solution would look like if there were both white images with transparent backgrounds and black images with transparent backgrounds.

As I already mentioned in the chat, this “problem” has existed for a very long time, since it affects transparent PNGs and WEBPs in exactly the same way.

If there isn't an issue open about this, it seems no one has had a problem with it so far.

1.

We should at least fix the composer dependency. Having a dependency on a library, the module usually doesn't need should be fixed here.

I already moved it to suggest for that reason

2.

Add a warning if it isn't installed?

Yes that would make sense in both submodules, not the main module

3. CSS point is nothing we should solve at all or not now, good enough

4. FYI: The svg sanitization controller only gets a route from both submodules so it's not used / available in the main module which is good.

Good enough and ready for merging once (2) is added.

Alright added the requirements hook for svg_image_field submodule. The other already requires "svg_image" which has a hard dependency on that library.

Please do a final review!

Thanks @grevil! RTBC!