Clumsy

I find it odd that you are criticizing Gedshow when it is the other plugin that prevents the uploading of the Gedcom. Surely you should raise this with the author of the other plugin?

Please explain the comment “problems which appear and disappear when using double versus single quotes to provide shortcode parameters again seem to be unnecessary complications in usage.”

If you are going to operate in a mixed environment such as what exists in the world of WordPress plugins, and you can code such that others cause problems for what you are doing, or if it is possible to code such that you can co-exists, and you chose to simple take the approach that it’s the other plugins problem, you are doing a great disservice to anyone who might benefit from your plugin. Clearly, as I have indicated and it is demonstrated by another plugin which processes the exact same GEDCOM file which your plugin does not, it is not really the problem of MLA, I for one, (who have been doing IT since 1970 an a wide variety of environments) consider the issue yours, not theirs.

Regarding the double quote, single quote comment I had your shortcode coded with double quotes on the parameters and failed to get it to function (after getting past the MLA problem by de-activating MLA). I changed the shortcode to use single quotes and obtained a proper display. I won’t swear to the problem, I’m just reporting, honestly, what I experienced.

Not everything I said about your plugin was negative, or I wouldn’t have given it a 3, it would have been lower. Compatibility in WordPress requires giving grace to others and not just saying it’s their problem not mine. If it doesn’t have to be anyone’s problem, why leave your code the way it is and make the problem an issue for the user?

MLA doesn’t prevent RootsPersona on the same install from uploading the same file; that, to me, makes the problem more yours than theirs. Why don’t you research their code (also open-source) and fix the problem in your plugin so the conflict doesn’t exist?

I have just replied to this support topic, giving a solution for the problem with Media Library Assistant:

Unable to upload .ged file | www.ads-software.com

If you have trouble with that solution, open a topic in the MLA forum or add a post to the existing topic. Thanks!

Again, the problem, IMHO does not belong to MLA, although there may be some aspect where they might improve their own code too.

Your code in GedShow rejects first with the error “You must upload a gedcom file with an extension of ged (in lowercase). If your WordPress install is multisite please see the Gedshow FAQ.” based on your use of

$file_type = wp_check_filetype($ged);
 	if (($file_type['ext'] !== 'ged')) 

on lines 337-338 of csgen.php because you do not supply the (second) mimes parameter, leaving the mime type processing to become WordPress default processes. WordPress does not have a ‘ged’ type, so somebody else needs to provide it, since you don’t do it yourself. This requires some other facility to do it for you. MLA is one such facility as described by David Lingren, above. File Upload Types is another such plugin facility. Disabling MLA and using File Upload Types is one answer, but then I lose MLA and have the hassle I identified in my OP.

Adding ‘ged’ as a MLA filetype still has problems because your next action on line 344 is

$uploaded = media_handle_upload('ged_upload', 0);

If I add it to MLA as David identifies, I get past the first error (which was, I believe, previously addressed in https://www.ads-software.com/support/topic/trouble-with-a-ged-upload/) and I get the new error ‘Error uploading file: Sorry, you are not allowed to upload this file type.‘ from line 344. The reason for this is that media_handle_upload() calls wp_handle_upload() which again checks for a valid mime type, but differently since MLA with a ged mime-type gets past the 337/338 check but not the 344 line.

The other reason for this is that again an opportunity exists to supply an override parameter to allow for the ‘ged’ mime-type which you chose not to supply on 344. (You supply a 0, taken as ‘false’, in place of an overrides array which could (should) include a mimes override. [See https://developer.www.ads-software.com/reference/functions/wp_handle_upload/%5D)

If your code were to simply supply the overrides using parameters which you are not using I suspect that not only would the conflict go away, but the need for external plugins to supply a ‘ged’ mime-type for your use would not exist.

Re: MLA, a note to David Lingren, I suggest you may want to look at the detail docs for _wp_handle_upload to see if you are hooking all of the mime-type verification hooks. But I also suggest that GedShow should have supplied the mime overrides to begin with and there would be no need for users to work around the problem.

Another alternative, which works, is to bypass some of the WP build in functions and read the file yourself with fopen(), but that is another whole strategy which you’ve obviously chosen not to follow. As is, with both GedShow and MLA in play, I have to disable MLA every time I want to upload a new, or updated, GEDCOM file for processing. Not cool.

Whose problem is it? Right now it belongs to anyone who wants to use both plugins.

• This reply was modified 1 year, 7 months ago by joevansteen.
• This reply was modified 1 year, 7 months ago by joevansteen.

@joevansteen Thank you for the detailed analysis of my code. I will look at what you suggest. However, if you had looked at the code at line 274 you would see that I do add the .ged as a mime type.

@colinsp I appreciate your patience with me. I am now of the opinion that your code is probably technically correct, although using the parameters on the calls instead of depending on the filter would probably bypass the problem with MLA, and anyone else who might have a conflict. You force the use of the upload_mimes filter twice in your logic, which should be okay, but it exposes the following.

@dglingren I have also looked at your code and see that you also apply the same upload_mimes filter to inject the mime-types which have been identified as per your instructions. However, in class-mla-mime-types.php in the mla_upload_mimes_filter (367-424) function you have a set of logic which appears to function on a “first-time” basis, and on subsequent calls it simply returns the mime-type array which was provided on the call, rather than adding supplementary items, again, to the list provided as the input array. This first time type behavior seems to match the results which I get when I execute GedShow logic with a ‘ged‘ mime-type specified as an MLA override type as you described in your solution linked above.

From my inspection of this same code, you also, on the first call, seem to ignore the mime-types provided as an input parameter and instead return a new mime-type array built to match your own extended list. That would also match the behavior where if a new mime-type (not identified to MLA but added by others, GedShow, or the File Upload Types plugin, gets ignored, assuming my reading of your code is correct. This would identify the problem behavior I’ve documented as well as problems previously identified by others regarding incompatibility between the two programs.

• If MLA is not present, the ‘ged’ type supplied via another plugin’s upload_mimes filter (GedShow, File Upload Types, etc.) works.
• When ‘ged’ is identified as a mime-type by others, but not MLA, the first check fails.
• When ‘ged’ is identified to MLA the first check works, the second check fails
• Bypassing the requirement and providing the ‘ged’ mime on the two calls should also work, but shouldn’t be required since WP defined practice provided for either as a programming choice. Defensive programming might suggest whether GedShow should be changed or not.
• Bypassing some of the WP functions and using PHP directly to open the file also works.

I really don’t have the time of patience to do a lot more of this, but changes to the code don’t seem too big for either, or both, of you to try; and from the GedShow trouble ticket history I don’t seem to be the only one affected who desires to use both plugins.

I hate WordPress. It’s programming in a totally chaotic environment. Yet, it seems to be a bit like democracy, the best of a bad lot of choices; and it seems such an appropriate tool from the other side of the screen. Yin/Yang.

• This reply was modified 1 year, 7 months ago by joevansteen. Reason: "Best" not "Beast" of bad choices

@joevansteen Thanks for the work that you did. I have taken onboard what you have suggested. As a result I have been working on the code today and am currently testing the resulting code. It has thrown up another issue I am still to get to the bottom of. As a result v2.0.18, when released, will work fine with no changes required to MLA (well it does in my test environment). I wrote the plugin as what I was using, Genealone, disappeared some years ago for myself and a couple of friends wanted copies hence releasing it here.

@colinsp You are welcome, and I apologize for pinning the blame on you in my opening salvo. I’m looking forward to v2.0.18 and will be happy to upgrade my rating on arrival.

I sincerely appreciate your response to the concern about defensive programming whenever possible and listening to the concerns of your users.

Thanks, @joevansteen for your extensive investigation and detailed comments. It has been several years since I looked at this part of the MLA code but I will go back to it and see how I can improve things. Since this is a support problem, not a review comment, I will post my results in the support topic Colin recently started: Unable to upload .ged file | www.ads-software.com. I will also post a note here when I have progress to report.

@dglingren?You are welcome. At first I thought some of the behavior was random based on the order in which co-equal upload_mimes filters were executed. However, I also see that you have set a maximum high priority on your filter, making almost always assured to be the last filter executed, thus ensuring the behavior specified above. I seriously doubt many other plugins specify as high a priority as yours. I don’t know why you coded it this way, but in this form, if my analysis is correct, your plugin will conflict with every other plugin dependent on the upload_mimes filter that tries to register and use a file extension which you do not have in your own tables.

I have uploaded a new MLA Development Version dated 20230309 that recognizes the MIME Type added by Gedshow and avoids the need to add the type manually. You can find step-by-step instructions for using the Development Version in this earlier topic:

PHP Warning on media upload with Polylang

Once the Development Version is installed you can retest the handling of the .ged MIME type and file extension added by the Gedshow plugin.

The fix will be part of my next MLA version. Thanks for inspiring this MLA improvement. Thanks as well for continuing the discussion in the MLA support topic you started:

Mime type handling | www.ads-software.com