Project

General

Profile

Feature #2092

Create upstream patch for WP Shadowbox plugin

Added by Boone Gorges over 6 years ago. Updated about 3 years ago.

Status:
Rejected
Priority name:
Low
Category name:
WordPress Plugins
Target version:
Start date:
2012-09-03
Due date:
% Done:

0%

Estimated time:

Description

Dom - In https://github.com/castiron/cac/commit/91cd47ba019eca8ed435c2c8dbae37e023d3a437 and https://github.com/castiron/cac/commit/67fa2e021ab73eab5f99f97c4d291c8f5aa4a489 you updated the WP Shadowbox plugin to use a shared copy of the library files, to avoid permissions errors and redundancy on multisite installations like ours. I would like to package a version of these changes into a patch that we can then send along to sivel, the plugin author.

The basic strategy should remain the same: on multisite, look for a shared library location (wp-content/shared_lib is fine with me, though perhaps 'lib' rather than 'shared_lib' would be a bit more predictable). But a few additional things would make the patch even better suited for the distribution plugin:

1) More graceful fallbacks. Instead of requiring that the shared lib be used in multisite, instead you should first try to use the shared_lib (maybe using file_exists()), and then fall back to the default blogs.dir location if it can't be found. Alternatively, because doing this check on every pageload requires some overhead, you might consider an admin setting that appears when multisite is enabled. Not sure what that would look like. Think about it and tell me if you have any brilliant ideas :)

2) Consider abstracting the library location. Right now it's scattered in a couple places through the plugin, but could easily be abstracted to a single method of the main plugin class.

3) Use apply_filters() just before returning the library location, so that admins can have total control over the location (eg, if they have a preferred location for storing shared library files in their MS installation).

To make the patch properly, get an svn checkout of the plugin http://plugins.svn.wordpress.org/shadowbox-js/trunk/ on a clean WP install. Then use svn diff to generate a patch file.

This is low priority. Do it when you have a few minutes downtime :)

History

#1 Updated by Boone Gorges over 6 years ago

  • Tracker changed from Bug to Feature

#2 Updated by Dominic Giglio over 6 years ago

1.) I like this idea, it's the best of all worlds. It keeps sivel's original code very similar to the way it currently is but allows a centralized src dir if that's what an admin prefers. My experience with admin settings in the past has not been encouraging. Because this is so low priority right now I think I might just leave that decision up to sivel. I will make that final decision once I get into the code a little more. If a simple solution presents itself, I'll do it.

2.) All I could think the whole time I was editing was that all the src dir code really NEEDED to be extracted and refactored. I will definitely make that a central part of the patch.

I'm going to look through all the point releases now, just like I did leading up to 1.4.3, and I'll update here again when I find a few hours to slip in between our more pressing issues.

#3 Updated by Boone Gorges about 4 years ago

  • Category name set to WordPress Plugins

#4 Updated by Boone Gorges about 3 years ago

  • Status changed from Assigned to Rejected

The plugin in question hasn't been updated in over 4 years, and is owned by a person who I know is not actively involved in the WordPress world anymore. As I don't want to adopt the plugin myself, I'm going to close this ticket, with the caveat that we can revisit the upstream improvements if/when the plugin ever comes back to life.

Also available in: Atom PDF