MyBB Community Forums

Full Version: Attachment: a bug and a mod to allow multi-attachment selection
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
Pages: 1 2 3
First, the bug (1.8.14).  The situation: one has uploaded an attachment for a new/edit post and one has selected another file to be uploaded but "Add Attachment to upload the second attachment has not been clicked yet.  If at that point, one clicks on "Remove Attachment", the selected attachment will be uploaded.

Now an improvement.  I was interested in allowing multi-attachments adds.  I found a plugin that added that capability, but it also added a number of other features.  I found the plugin unusable because of various bugs, many of which were related to features other than supporting multifile upload.  I implemented the following fairly straight-forward changes to allow the multi-file uploads.  If anyone wants to try it, and provide feedback, at some point I may turn it into a plugin.

Uploaded files come into the *.php code via the variables "$_FILES['attachment'][$field]", where $field has various values like 'name' and 'size' for various aspects of the uploaded file. For multi-file uploads, those variables become arrays instead of singular values.  Where those variables are used, the following code segment must be inserted.

		$fields = array ('name', 'type', 'tmp_name', 'error', 'size');
		if (is_array($_FILES['attachment']['name']))
		{
			// Already in multi-attachment array format
			$attachments = $_FILES;
		}
		else
		{
			// Convert original-style non-array $_FILES['attachment'][$field] to array format in $attachments
			$attachments = array('attachment' => array());
			foreach ($fields as $field)
				$attachments['attachment'][$field] = array($_FILES['attachment'][$field]);
		}
		foreach ($attachments['attachment']['name'] as $key => $name)
		{
		    // Convert array $attachments['attachment'][$field][$key] to non-array format in $FILE
		    $FILE = array('attachment' => array());				
		    foreach ($fields as $field)
		        $FILE['attachment'][$field] = $attachments['attachment'][$field][$key];

Step 1: The affected files are newthread.php, editpost.php and newreply.php, around line 225, give or take, in each.  In each file, insert the code before the first "if" statements that process the attachment (and add another closing curly brace for the added "foreach" statement).

Step 2: In that "if" staement, chnage all references to "$_FILES" to "$FILE".


Example: in newthread.php, the new code segment would look like this:
[attachment=40094]

Step 4: Change the "input" tag in template "post_attachments_new" to have name "attachment[]" (braces added) and the keyword "multiple".
<input type="file" name="attachment[]" size="30" class="fileupload" multiple/>

Older browsers may still be limited to single attachment uploads but it still should work for that.
Great! I would like to see this feature Smile also something like box with drag and drop mouse support for easier uploading (phpbb style).

EDIT: can you please provide full diff of these three modified files? Or enclose them into attachment?
Thank You for reporting & working with the code. it will be checked by the Developers.
Also would be nice to add checkboxes for multiple deleting of attachments and show the names of selected files for upload instead of the number. Great job, thank you!
There is a plugin that I believe does all of those things, https://community.mybb.com/mods.php?action=view&pid=104 . I tried it, but found it too buggy. The thing I really wanted was multi-attach and under the KISS principle, when I found how simple it was to implement multi-attach, I figured that was a more reliable way to go than deal with the plugin (at the cost of not getting those "toys" which were not crucial to me). I found the showing of the number of selected files thing interesting as that is not implemented in the MyBB stuff. That message is produced the the browser itself and some quick research did NOT reveal anyway to change that behavior. So getting that list of files is not a simple coding adjustment.
This plugin is really bugged. Do you plan to release your plugin for multiple attachment?

Reply to your last PM - everything is working correctly, thank you!
Have you though about using a library ? Since 1.9+ are more about enhancing than rewriting, we could go with implementing a library than handles this. It could be useful for future developers that wish to implement similar behavior within their modifications. I'm talking about something simple as:
https://github.com/FineUploader/fine-uploader

I think people would appreciate an implementation like this, especially if we are getting PM attachments (iirc we were).
(2018-04-10, 03:09 PM)Omar G. Wrote: [ -> ]Have you though about using a library ? Since 1.9+ are more about enhancing than rewriting, we could go with implementing a library than handles this. It could be useful for future developers that wish to implement similar behavior within their modifications. I'm talking about something simple as:
https://github.com/FineUploader/fine-uploader

I think people would appreciate an implementation like this, especially if we are getting PM attachments (iirc we were).

I do not know what a "library" means in the context of the MybB code.  But when I ran into an "ugly" with the mutlti-attachments, I did rewrite the code to use a function.

The "ugly" was when there was a multi-attachment, the order the files were loaded was not consistent.  Probably there was a race ocndition of some sort.  So I added to the handling of the attachment a sort routine that causes the attachments to be loaded in filename sorted order (the primary use on my site is for loading phots of an event where the file names are the appropriate order).

The function is this (I put it in inc/functions_post.php).


/**
 * Process adding attachment(s) when the "Add Attachment" button is pressed.
 *
 * @param int $pid The ID of the post.
 * @param array $forumpermission The permissions for the forum.
 * @param string $attachwhere Search string "pid='$pid'" or "posthash='".$db->escape_string($mybb->get_input('posthash'))."'"
 * @param string $caller Where called from: "newthread", "newreply", or "editpost"
 */
function on_add_attachments($pid, $forumpermissions, $attachwhere, $caller)
{
	global $db, $mybb, $errors, $editdraftpid;
	if($forumpermissions['canpostattachments'] != 0)
	{
		//----------------------------------------------------------------------------------------
		// Code should work with either original single attachment or revised multi-attachment $_FILES format
		//----------------------------------------------------------------------------------------
		$fields = array ('name', 'type', 'tmp_name', 'error', 'size');
		if (is_array($_FILES['attachment']['name']))
		{
			// Multi-attachments, make array of attachments from $_FILES
			$attachments = array();
		   	foreach ($_FILES['attachment']['name'] as $key => $name)
			{
				$attach1 = array();
				foreach ($fields as $field)
					$attach1[$field] = $_FILES['attachment'][$field][$key];
				$attachments[] = $attach1;
			}
		}
		else
		{
			// Single attachment in original-style non-array $_FILES['attachment'][$field], make a single element array of attachments for that
			$attachments = array();
			$attachments[] = $_FILES['attachment'];
		}
		usort ($attachments, function($a,$b){ return strcmp($a['name'],$b['name']); }); 
		foreach ($attachments as $FILE)
		{
			if(!empty($FILE['name']) && !empty($FILE['type']))
			{
				if($FILE['size'] > 0)
				{
					$query = $db->simple_select("attachments", "aid", "filename='".$db->escape_string($FILE['name'])."' AND {$attachwhere}");
					$updateattach = $db->fetch_field($query, "aid");

					require_once MYBB_ROOT."inc/functions_upload.php";

					$update_attachment = true;
					if($updateattach <= 0 || !$mybb->get_input('updateattachment'))
					{
						$update_attachment = false;
					}
					if($caller == "editpost" && (!$mybb->usergroup['caneditattachments'] && !$forumpermissions['caneditattachments']))
					{
						$update_attachment = false;
					}

					$attachedfile = upload_attachment($FILE, $update_attachment);
				}
				else
				{
					$errors[] = $lang->error_uploadempty;
					$mybb->input['action'] = $caller;
				}
			}

			// Error with attachments - should use new inline errors?
			if(!empty($attachedfile['error']))
			{
				$errors[] = $attachedfile['error'];
				$mybb->input['action'] = $caller;
			}

		}
	}
	// If we were dealing with an attachment but didn't click 'Post Thread', force the new thread page again.
	if(!$mybb->get_input('submit'))
	{
		// SPMOD: it appear the following line is no longer needed, so it has been commented out
		// $editdraftpid = "<input type=\"hidden\" name=\"pid\" value=\"$pid\" />";
		$mybb->input['action'] = $caller;
	}
}

Now the changes to the three files involve just replacing a bunch of code with a function call (the code should be replaced with a function call even if there is no functional change, it makes the code "cleaner").  The line numbers may be off a bit because of other changes I have made in my code.

newthread.php
174,213c176,177
< 	// If there's an attachment, check it and upload it
< 	if($forumpermissions['canpostattachments'] != 0)
< 	{
< 		if(!empty($_FILES['attachment']['name']) && !empty($_FILES['attachment']['type']))
< 		{
< 			if($_FILES['attachment']['size'] > 0)
< 			{
< 				$query = $db->simple_select("attachments", "aid", "filename='".$db->escape_string($_FILES['attachment']['name'])."' AND {$attachwhere}");
< 				$updateattach = $db->fetch_field($query, "aid");
< 
< 				require_once MYBB_ROOT."inc/functions_upload.php";
< 
< 				$update_attachment = false;
< 				if($updateattach > 0 && $mybb->get_input('updateattachment'))
< 				{
< 					$update_attachment = true;
< 				}
< 				$attachedfile = upload_attachment($_FILES['attachment'], $update_attachment);
< 			}
< 			else
< 			{
< 				$errors[] = $lang->error_uploadempty;
< 				$mybb->input['action'] = "newthread";
< 			}
< 		}
< 	}
< 
< 	// Error with attachments - should use new inline errors?
< 	if(!empty($attachedfile['error']))
< 	{
< 		$errors[] = $attachedfile['error'];
< 		$mybb->input['action'] = "newthread";
< 	}
< 
< 	// If we were dealing with an attachment but didn't click 'Post Thread', force the new thread page again.
< 	if(!$mybb->get_input('submit'))
< 	{
< 		//$editdraftpid = "<input type=\"hidden\" name=\"pid\" value=\"$pid\" />";
< 		$mybb->input['action'] = "newthread";
< 	}
---
> 	// SPMOD: MA function call replaces a bunch of code
> 	on_add_attachments($pid, $forumpermissions, $attachwhere, "newthread");


editpost.php:
197,218c200,201
< 	// If there's an attachment, check it and upload it
< 	if($_FILES['attachment']['size'] > 0 && $forumpermissions['canpostattachments'] != 0)
< 	{
< 		$query = $db->simple_select("attachments", "aid", "filename='".$db->escape_string($_FILES['attachment']['name'])."' AND pid='{$pid}'");
< 		$updateattach = $db->fetch_field($query, "aid");
< 
< 		$update_attachment = false;
< 		if($updateattach > 0 && $mybb->get_input('updateattachment') && ($mybb->usergroup['caneditattachments'] || $forumpermissions['caneditattachments']))
< 		{
< 			$update_attachment = true;
< 		}
< 		$attachedfile = upload_attachment($_FILES['attachment'], $update_attachment);
< 	}
< 	if(!empty($attachedfile['error']))
< 	{
< 		eval("\$attacherror = \"".$templates->get("error_attacherror")."\";");
< 		$mybb->input['action'] = "editpost";
< 	}
< 	if(!isset($mybb->input['submit']))
< 	{
< 		$mybb->input['action'] = "editpost";
< 	}
---
> 	// SPMOD: MA function call replaces a bunch of code
> 	on_add_attachments($pid, $forumpermissions, "pid='{$pid}'", "editpost");

newpost.php
225,263c225,226
< 	// If there's an attachment, check it and upload it
< 	if($forumpermissions['canpostattachments'] != 0)
< 	{
< 		// If attachment exists..
< 		if(!empty($_FILES['attachment']['name']) && !empty($_FILES['attachment']['type']))
< 		{
< 			if($_FILES['attachment']['size'] > 0)
< 			{
< 				$query = $db->simple_select("attachments", "aid", "filename='".$db->escape_string($_FILES['attachment']['name'])."' AND {$attachwhere}");
< 				$updateattach = $db->fetch_field($query, "aid");
< 
< 				require_once MYBB_ROOT."inc/functions_upload.php";
< 
< 				$update_attachment = false;
< 				if($updateattach > 0 && $mybb->get_input('updateattachment'))
< 				{
< 					$update_attachment = true;
< 				}
< 				$attachedfile = upload_attachment($_FILES['attachment'], $update_attachment);
< 			}
< 			else
< 			{
< 				$errors[] = $lang->error_uploadempty;
< 				$mybb->input['action'] = "newreply";
< 			}
< 		}
< 	}
< 
< 	if(!empty($attachedfile['error']))
< 	{
< 		$errors[] = $attachedfile['error'];
< 		$mybb->input['action'] = "newreply";
< 	}
< 
< 	if(!$mybb->get_input('submit'))
< 	{
< 		eval("\$editdraftpid = \"".$templates->get("newreply_draftinput")."\";");
< 		$mybb->input['action'] = "newreply";
< 	}
---
> 	// SPMOD: MA function call replaces a bunch of code
> 	on_add_attachments($pid, $forumpermissions, $attachwhere, "newreply");
I'm talking about implementing the following:

(2015-08-13, 06:38 PM)Euan T Wrote: [ -> ]
  • UI Wise, the attachment system should support drag/drop with multiple uploads and a simple load button fallback for users without JavaScript. Uploads should be performed via AJAX.

I know it is a hassle but please try to update your pull request so that it can be tested.
I am sorry but I do not know what you are talking about. My post had two purposes. First, I found a bug and wanted to report it. Second I made a minor modification to the code that I thought added some useful functionality. Later I wrote a function so instead of around 60 lines of code being repeated in three places, it was all put into a function (which makes for better code).

You earlier spoke of a "library". Now you are calling what I wrote a "pull request" and saying it needs "updating". What is a "pull request"? I thought my reply gave complete and straightforward details describing the change I made. So, I have no idea what I can add (update) in addition to that.
Pages: 1 2 3