Merged ajax_upload branch (#3957).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@10977 e93f8b46-1217-0410-a6f0-8f06a7374b81

22 files changed, 442 insertions, 83 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 585ac5788..a4c804fd8 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -300,6 +300,16 @@ class ApplicationController < ActionController::Base
     render_404
   end
 
+  def find_attachments
+    if (attachments = params[:attachments]).present?
+      att = attachments.values.collect do |attachment|
+        Attachment.find_by_token( attachment[:token] ) if attachment[:token].present?
+      end
+      att.compact!
+    end
+    @attachments = att || []
+  end
+
   # make sure that the user is a member of the project (or admin) if project is private
   # used as a before_filter for actions that do not require any particular permission on the project
   def check_project_privacy
diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb
index 13871c323..53e0fd6d4 100644
--- a/app/controllers/attachments_controller.rb
+++ b/app/controllers/attachments_controller.rb
@@ -85,15 +85,17 @@ class AttachmentsController < ApplicationController
     @attachment = Attachment.new(:file => request.raw_post)
     @attachment.author = User.current
     @attachment.filename = params[:filename].presence || Redmine::Utils.random_hex(16)
+    saved = @attachment.save
 
-    if @attachment.save
-      respond_to do |format|
-        format.api { render :action => 'upload', :status => :created }
-      end
-    else
-      respond_to do |format|
-        format.api { render_validation_errors(@attachment) }
-      end
+    respond_to do |format|
+      format.js
+      format.api {
+        if saved
+          render :action => 'upload', :status => :created
+        else
+          render_validation_errors(@attachment)
+        end
+      }
     end
   end
 
@@ -101,9 +103,17 @@ class AttachmentsController < ApplicationController
     if @attachment.container.respond_to?(:init_journal)
       @attachment.container.init_journal(User.current)
     end
-    # Make sure association callbacks are called
-    @attachment.container.attachments.delete(@attachment)
-    redirect_to_referer_or project_path(@project)
+    if @attachment.container
+      # Make sure association callbacks are called
+      @attachment.container.attachments.delete(@attachment)
+    else
+      @attachment.destroy
+    end
+
+    respond_to do |format|
+      format.html { redirect_to_referer_or project_path(@project) }
+      format.js
+    end
   end
 
 private
diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb
index 16e3b7c45..82b74dda5 100644
--- a/app/controllers/messages_controller.rb
+++ b/app/controllers/messages_controller.rb
@@ -19,6 +19,7 @@ class MessagesController < ApplicationController
   menu_item :boards
   default_search_scope :messages
   before_filter :find_board, :only => [:new, :preview]
+  before_filter :find_attachments, :only => [:preview]
   before_filter :find_message, :except => [:new, :preview]
   before_filter :authorize, :except => [:preview, :edit, :destroy]
 
@@ -117,7 +118,6 @@ class MessagesController < ApplicationController
 
   def preview
     message = @board.messages.find_by_id(params[:id])
-    @attachements = message.attachments if message
     @text = (params[:message] || params[:reply])[:content]
     @previewed = message
     render :partial => 'common/preview'
diff --git a/app/controllers/previews_controller.rb b/app/controllers/previews_controller.rb
index 9594a9fe1..4ea945c64 100644
--- a/app/controllers/previews_controller.rb
+++ b/app/controllers/previews_controller.rb
@@ -16,12 +16,11 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 class PreviewsController < ApplicationController
-  before_filter :find_project
+  before_filter :find_project, :find_attachments
 
   def issue
     @issue = @project.issues.find_by_id(params[:id]) unless params[:id].blank?
     if @issue
-      @attachements = @issue.attachments
       @description = params[:issue] && params[:issue][:description]
       if @description && @description.gsub(/(\r?\n|\n\r?)/, "\n") == @issue.description.to_s.gsub(/(\r?\n|\n\r?)/, "\n")
         @description = nil
@@ -37,7 +36,6 @@ class PreviewsController < ApplicationController
   def news
     if params[:id].present? && news = News.visible.find_by_id(params[:id])
       @previewed = news
-      @attachments = news.attachments
     end
     @text = (params[:news] ? params[:news][:description] : nil)
     render :partial => 'common/preview'
diff --git a/app/controllers/wiki_controller.rb b/app/controllers/wiki_controller.rb
index fd7076024..115f61157 100644
--- a/app/controllers/wiki_controller.rb
+++ b/app/controllers/wiki_controller.rb
@@ -37,6 +37,7 @@ class WikiController < ApplicationController
   before_filter :find_existing_or_new_page, :only => [:show, :edit, :update]
   before_filter :find_existing_page, :only => [:rename, :protect, :history, :diff, :annotate, :add_attachment, :destroy, :destroy_version]
   accept_api_auth :index, :show, :update, :destroy
+  before_filter :find_attachments, :only => [:preview]
 
   helper :attachments
   include AttachmentsHelper
@@ -293,7 +294,7 @@ class WikiController < ApplicationController
     # page is nil when previewing a new page
     return render_403 unless page.nil? || editable?(page)
     if page
-      @attachements = page.attachments
+      @attachments += page.attachments
       @previewed = page.content
     end
     @text = params[:content][:text]
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 8173bdb44..1a0c9ea5f 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -597,8 +597,9 @@ module ApplicationHelper
 
   def parse_inline_attachments(text, project, obj, attr, only_path, options)
     # when using an image link, try to use an attachment, if possible
-    if options[:attachments] || (obj && obj.respond_to?(:attachments))
-      attachments = options[:attachments] || obj.attachments
+    if options[:attachments].present? || (obj && obj.respond_to?(:attachments))
+      attachments = options[:attachments] || []
+      attachments += obj.attachments if obj
       text.gsub!(/src="([^\/"]+\.(bmp|gif|jpg|jpe|jpeg|png))"(\s+alt="([^"]*)")?/i) do |m|
         filename, ext, alt, alttext = $1.downcase, $2, $3, $4
         # search for the picture in attachments
diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index a1c232d16..6fa079c91 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -154,11 +154,19 @@ class Attachment < ActiveRecord::Base
   end
 
   def visible?(user=User.current)
-    container && container.attachments_visible?(user)
+    if container_id
+      container && container.attachments_visible?(user)
+    else
+      author == user
+    end
   end
 
   def deletable?(user=User.current)
-    container && container.attachments_deletable?(user)
+    if container_id
+      container && container.attachments_deletable?(user)
+    else
+      author == user
+    end
   end
 
   def image?
diff --git a/app/views/attachments/_form.html.erb b/app/views/attachments/_form.html.erb
index e47c288a9..092f68244 100644
--- a/app/views/attachments/_form.html.erb
+++ b/app/views/attachments/_form.html.erb
@@ -1,18 +1,28 @@
+<span id="attachments_fields">
 <% if defined?(container) && container && container.saved_attachments %>
   <% container.saved_attachments.each_with_index do |attachment, i| %>
-    <span class="icon icon-attachment" style="display:block; line-height:1.5em;">
-      <%= h(attachment.filename) %> (<%= number_to_human_size(attachment.filesize) %>)
-      <%= hidden_field_tag "attachments[p#{i}][token]", "#{attachment.id}.#{attachment.digest}" %>
+    <span id="attachments_p<%= i %>">
+      <%= text_field_tag("attachments[p#{i}][filename]", attachment.filename, :class => 'filename') +
+          text_field_tag("attachments[p#{i}][description]", attachment.description, :maxlength => 255, :placeholder => l(:label_optional_description), :class => 'description') +
+          link_to('&nbsp;'.html_safe, attachment_path(attachment, :attachment_id => "p#{i}", :format => 'js'), :method => 'delete', :remote => true, :class => 'remove-upload') %>
+      <%= hidden_field_tag "attachments[p#{i}][token]", "#{attachment.token}" %>
     </span>
   <% end %>
 <% end %>
-<span id="attachments_fields">
-  <span>
-    <%= file_field_tag 'attachments[1][file]', :id => nil, :class => 'file',
-          :onchange => "checkFileSize(this, #{Setting.attachment_max_size.to_i.kilobytes}, '#{escape_javascript(l(:error_attachment_too_big, :max_size => number_to_human_size(Setting.attachment_max_size.to_i.kilobytes)))}');"  -%>
-    <%= text_field_tag 'attachments[1][description]', '', :id => nil, :class => 'description', :maxlength => 255, :placeholder => l(:label_optional_description) %>
-    <%= link_to_function(image_tag('delete.png'), 'removeFileField(this)', :title => (l(:button_delete))) %>
-  </span>
 </span>
-<span class="add_attachment"><%= link_to l(:label_add_another_file), '#', :onclick => 'addFileField(); return false;', :class => 'add_attachment' %>
-(<%= l(:label_max_size) %>: <%= number_to_human_size(Setting.attachment_max_size.to_i.kilobytes) %>)</span>
+<span class="add_attachment">
+<%= file_field_tag 'attachments_files',
+      :id => nil,
+      :multiple => true,
+      :onchange => 'addInputFiles(this);',
+      :data => {
+        :max_file_size => Setting.attachment_max_size.to_i.kilobytes,
+        :max_file_size_message => l(:error_attachment_too_big, :max_size => number_to_human_size(Setting.attachment_max_size.to_i.kilobytes)),
+        :max_concurrent_uploads => Redmine::Configuration['max_concurrent_ajax_uploads'].to_i,
+        :upload_path => uploads_path(:format => 'js'),
+        :description_placeholder => l(:label_optional_description)
+      } %>
+(<%= l(:label_max_size) %>: <%= number_to_human_size(Setting.attachment_max_size.to_i.kilobytes) %>)
+</span>
+
+<%= javascript_include_tag 'attachments' %>
diff --git a/app/views/attachments/destroy.js.erb b/app/views/attachments/destroy.js.erb
new file mode 100644
index 000000000..3cfb5845f
--- /dev/null
+++ b/app/views/attachments/destroy.js.erb
@@ -0,0 +1 @@
+$('#attachments_<%= j params[:attachment_id] %>').remove();
diff --git a/app/views/attachments/upload.js.erb b/app/views/attachments/upload.js.erb
new file mode 100644
index 000000000..04e30b569
--- /dev/null
+++ b/app/views/attachments/upload.js.erb
@@ -0,0 +1,9 @@
+var fileSpan = $('#attachments_<%= j params[:attachment_id] %>');
+$('<input>', { type: 'hidden', name: 'attachments[<%= j params[:attachment_id] %>][token]' } ).val('<%= j @attachment.token %>').appendTo(fileSpan);
+fileSpan.find('a.remove-upload')
+  .attr({
+    "data-remote": true,
+    "data-method": 'delete',
+    href: '<%= j attachment_path(@attachment, :attachment_id => params[:attachment_id], :format => 'js') %>'
+  })
+  .off('click');
diff --git a/app/views/common/_preview.html.erb b/app/views/common/_preview.html.erb
index fd95f1188..90d83ce8c 100644
--- a/app/views/common/_preview.html.erb
+++ b/app/views/common/_preview.html.erb
@@ -1,3 +1,3 @@
 <fieldset class="preview"><legend><%= l(:label_preview) %></legend>
-<%= textilizable @text, :attachments => @attachements, :object => @previewed %>
+<%= textilizable @text, :attachments => @attachments, :object => @previewed %>
 </fieldset>
diff --git a/app/views/previews/issue.html.erb b/app/views/previews/issue.html.erb
index 60ad2a0ad..a88bec6fc 100644
--- a/app/views/previews/issue.html.erb
+++ b/app/views/previews/issue.html.erb
@@ -1,11 +1,11 @@
 <% if @notes %>
   <fieldset class="preview"><legend><%= l(:field_notes) %></legend>
-    <%= textilizable @notes, :attachments => @attachements, :object => @issue %>
+    <%= textilizable @notes, :attachments => @attachments, :object => @issue %>
   </fieldset>
 <% end %>
 
 <% if @description %>
   <fieldset class="preview"><legend><%= l(:field_description) %></legend>
-    <%= textilizable @description, :attachments => @attachements, :object => @issue %>
+    <%= textilizable @description, :attachments => @attachments, :object => @issue %>
   </fieldset>
 <% end %>
diff --git a/config/configuration.yml.example b/config/configuration.yml.example
index 2e01eac4f..59527f874 100644
--- a/config/configuration.yml.example
+++ b/config/configuration.yml.example
@@ -188,6 +188,9 @@ default:
   #
   rmagick_font_path:
 
+  # Maximum number of simultaneous AJAX uploads
+  #max_concurrent_ajax_uploads: 2
+
 # specific configuration options for production environment
 # that overrides the default ones
 production:
diff --git a/lib/redmine/configuration.rb b/lib/redmine/configuration.rb
index 99ca92b36..ce1ee38aa 100644
--- a/lib/redmine/configuration.rb
+++ b/lib/redmine/configuration.rb
@@ -20,7 +20,8 @@ module Redmine
 
     # Configuration default values
     @defaults = {
-      'email_delivery' => nil
+      'email_delivery' => nil,
+      'max_concurrent_ajax_uploads' => 2
     }
 
     @config = nil
diff --git a/public/images/hourglass.png b/public/images/hourglass.png
new file mode 100644
index 000000000..57b03ce7a
--- /dev/null
+++ b/public/images/hourglass.png
diff --git a/public/javascripts/application.js b/public/javascripts/application.js
index 35b24ad05..e5886a666 100644
--- a/public/javascripts/application.js
+++ b/public/javascripts/application.js
@@ -290,40 +290,6 @@ function submit_query_form(id) {
   $('#'+id).submit();
 }
 
-var fileFieldCount = 1;
-function addFileField() {
-  var fields = $('#attachments_fields');
-  if (fields.children().length >= 10) return false;
-  fileFieldCount++;
-  var s = fields.children('span').first().clone();
-  s.children('input.file').attr('name', "attachments[" + fileFieldCount + "][file]").val('');
-  s.children('input.description').attr('name', "attachments[" + fileFieldCount + "][description]").val('');
-  fields.append(s);
-}
-
-function removeFileField(el) {
-  var fields = $('#attachments_fields');
-  var s = $(el).parents('span').first();
-  if (fields.children().length > 1) {
-    s.remove();
-  } else {
-    s.children('input.file').val('');
-    s.children('input.description').val('');
-  }
-}
-
-function checkFileSize(el, maxSize, message) {
-  var files = el.files;
-  if (files) {
-    for (var i=0; i<files.length; i++) {
-      if (files[i].size > maxSize) {
-        alert(message);
-        el.value = "";
-      }
-    }
-  }
-}
-
 function showTab(name) {
   $('div#content .tab-content').hide();
   $('div.tabs a').removeClass('selected');
@@ -579,8 +545,8 @@ function warnLeavingUnsaved(message) {
 };
 
 $(document).ready(function(){
-  $('#ajax-indicator').bind('ajaxSend', function(){
-    if ($('.ajax-loading').length == 0) {
+  $('#ajax-indicator').bind('ajaxSend', function(event, xhr, settings){
+    if ($('.ajax-loading').length == 0 && settings.contentType != 'application/octet-stream') {
       $('#ajax-indicator').show();
     }
   });
@@ -607,5 +573,10 @@ function addFormObserversForDoubleSubmit() {
   });
 }
 
+function blockEventPropagation(event) {
+  event.stopPropagation();
+  event.preventDefault();
+}
+
 $(document).ready(hideOnLoad);
 $(document).ready(addFormObserversForDoubleSubmit);
diff --git a/public/javascripts/attachments.js b/public/javascripts/attachments.js
new file mode 100644
index 000000000..fa5722723
--- /dev/null
+++ b/public/javascripts/attachments.js
@@ -0,0 +1,189 @@
+/* Redmine - project management software
+   Copyright (C) 2006-2012  Jean-Philippe Lang */
+
+function addFile(inputEl, file, eagerUpload) {
+
+  if ($('#attachments_fields').children().length < 10) {
+
+    var attachmentId = addFile.nextAttachmentId++;
+
+    var fileSpan = $('<span>', { id: 'attachments_' + attachmentId });
+    
+    fileSpan.append(
+        $('<input>', { type: 'text', 'class': 'filename readonly', name: 'attachments[' + attachmentId + '][filename]', readonly: 'readonly'} ).val(file.name),
+        $('<input>', { type: 'text', 'class': 'description', name: 'attachments[' + attachmentId + '][description]', maxlength: 255, placeholder: $(inputEl).data('description-placeholder') } ).toggle(!eagerUpload),
+        $('<a>&nbsp</a>').attr({ href: "#", 'class': 'remove-upload' }).click(removeFile).toggle(!eagerUpload)
+    ).appendTo('#attachments_fields');
+
+    if(eagerUpload) {
+      ajaxUpload(file, attachmentId, fileSpan, inputEl);
+    }
+
+    return attachmentId;
+  }
+  return null;
+}
+
+addFile.nextAttachmentId = 1;
+
+function ajaxUpload(file, attachmentId, fileSpan, inputEl) {
+
+  function onLoadstart(e) {
+    fileSpan.removeClass('ajax-waiting');
+    fileSpan.addClass('ajax-loading');
+    $('input:submit', $(this).parents('form')).attr('disabled', 'disabled');    
+  }
+
+  function onProgress(e) {
+    if(e.lengthComputable) {
+      this.progressbar( 'value', e.loaded * 100 / e.total );
+    }
+  }
+
+  function actualUpload(file, attachmentId, fileSpan, inputEl) {
+
+    ajaxUpload.uploading++;
+
+    uploadBlob(file, $(inputEl).data('upload-path'), attachmentId, {
+        loadstartEventHandler: onLoadstart.bind(progressSpan),
+        progressEventHandler: onProgress.bind(progressSpan)
+      })
+      .done(function(result) {
+        progressSpan.progressbar( 'value', 100 ).remove();
+        fileSpan.find('input.description, a').css('display', 'inline-block');
+      })
+      .fail(function(result) {
+        progressSpan.text(result.statusText);
+      }).always(function() {
+        ajaxUpload.uploading--;
+        fileSpan.removeClass('ajax-loading');
+        var form = fileSpan.parents('form');
+        if (form.queue('upload').length == 0 && ajaxUpload.uploading == 0) {
+          $('input:submit', form).removeAttr('disabled');
+        }
+        form.dequeue('upload');
+      });
+  }
+
+  var progressSpan = $('<div>').insertAfter(fileSpan.find('input.filename'));
+  progressSpan.progressbar();
+  fileSpan.addClass('ajax-waiting');
+
+  var maxSyncUpload = $(inputEl).data('max-concurrent-uploads');
+
+  if(maxSyncUpload == null || maxSyncUpload <= 0 || ajaxUpload.uploading < maxSyncUpload)
+    actualUpload(file, attachmentId, fileSpan, inputEl);
+  else
+    $(inputEl).parents('form').queue('upload', actualUpload.bind(this, file, attachmentId, fileSpan, inputEl));
+}
+
+ajaxUpload.uploading = 0;
+
+function removeFile() {
+  $(this).parent('span').remove();
+  return false;
+}
+
+function uploadBlob(blob, uploadUrl, attachmentId, options) {
+
+  var actualOptions = $.extend({
+    loadstartEventHandler: $.noop,
+    progressEventHandler: $.noop
+  }, options);
+
+  uploadUrl = uploadUrl + '?attachment_id=' + attachmentId;
+  if (blob instanceof window.File) {
+    uploadUrl += '&filename=' + encodeURIComponent(blob.name);
+  }
+
+  return $.ajax(uploadUrl, {
+    type: 'POST',
+    contentType: 'application/octet-stream',
+    beforeSend: function(jqXhr) {
+      jqXhr.setRequestHeader('Accept', 'application/js');
+    },
+    xhr: function() {
+      var xhr = $.ajaxSettings.xhr();
+      xhr.upload.onloadstart = actualOptions.loadstartEventHandler;
+      xhr.upload.onprogress = actualOptions.progressEventHandler;
+      return xhr;
+    },
+    data: blob,
+    cache: false,
+    processData: false
+  });
+}
+
+function addInputFiles(inputEl) {
+  var clearedFileInput = $(inputEl).clone().val('');
+
+  if (inputEl.files) {
+    // upload files using ajax
+    uploadAndAttachFiles(inputEl.files, inputEl);
+    $(inputEl).remove();
+  } else {
+    // browser not supporting the file API, upload on form submission
+    var attachmentId;
+    var aFilename = inputEl.value.split(/\/|\\/);
+    attachmentId = addFile(inputEl, { name: aFilename[ aFilename.length - 1 ] }, false);
+    if (attachmentId) {
+      $(inputEl).attr({ name: 'attachments[' + attachmentId + '][file]', style: 'display:none;' }).appendTo('#attachments_' + attachmentId);
+    }
+  } 
+
+  clearedFileInput.insertAfter('#attachments_fields');
+}
+
+function uploadAndAttachFiles(files, inputEl) {
+
+  var maxFileSize = $(inputEl).data('max-file-size');
+  var maxFileSizeExceeded = $(inputEl).data('max-file-size-message');
+
+  var sizeExceeded = false;
+  $.each(files, function() {
+    if (this.size && maxFileSize && this.size > parseInt(maxFileSize)) {sizeExceeded=true;}
+  });
+  if (sizeExceeded) {
+    window.alert(maxFileSizeExceeded);
+  } else {
+    $.each(files, function() {addFile(inputEl, this, true);});
+  }
+}
+
+function handleFileDropEvent(e) {
+
+  $(this).removeClass('fileover');
+  blockEventPropagation(e);
+
+  if ($.inArray('Files', e.dataTransfer.types) > -1) {
+
+    uploadAndAttachFiles(e.dataTransfer.files, $('input:file[name=attachments_files]'));
+  }
+}
+
+function dragOverHandler(e) {
+  $(this).addClass('fileover');
+  blockEventPropagation(e);
+}
+
+function dragOutHandler(e) {
+  $(this).removeClass('fileover');
+  blockEventPropagation(e);
+}
+
+function setupFileDrop() {
+  if (window.File && window.FileList && window.ProgressEvent && window.FormData) {
+
+    $.event.fixHooks.drop = { props: [ 'dataTransfer' ] };
+
+    $('form div.box').has('input:file').each(function() {
+      $(this).on({
+          dragover: dragOverHandler,
+          dragleave: dragOutHandler,
+          drop: handleFileDropEvent
+      });
+    });
+  }
+}
+
+$(document).ready(setupFileDrop);
diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css
index 57c1cb912..862bc2613 100644
--- a/public/stylesheets/application.css
+++ b/public/stylesheets/application.css
@@ -527,9 +527,16 @@ fieldset#notified_events .parent { padding-left: 20px; }
 span.required {color: #bb0000;}
 .summary {font-style: italic;}
 
-#attachments_fields input.description {margin-left: 8px; width:340px;}
+#attachments_fields input.description {margin-left:4px; width:340px;}
 #attachments_fields span {display:block; white-space:nowrap;}
-#attachments_fields img {vertical-align: middle;}
+#attachments_fields input.filename {border:0; height:1.8em; width:250px; color:#555; background-color:inherit; background:url(../images/attachment.png) no-repeat 1px 50%; padding-left:18px;}
+#attachments_fields .ajax-waiting input.filename {background:url(../images/hourglass.png) no-repeat 0px 50%;}
+#attachments_fields .ajax-loading input.filename {background:url(../images/loading.gif) no-repeat 0px 50%;}
+#attachments_fields div.ui-progressbar { width: 100px; height:14px; margin: 2px 0 -5px 8px; display: inline-block; }
+a.remove-upload {background: url(../images/delete.png) no-repeat 1px 50%; width:1px; display:inline-block; padding-left:16px;}
+a.remove-upload:hover {text-decoration:none !important;}
+
+div.fileover { background-color: lavender; }
 
 div.attachments { margin-top: 12px; }
 div.attachments p { margin:4px 0 2px 0; }
diff --git a/test/functional/attachments_controller_test.rb b/test/functional/attachments_controller_test.rb
index ac66a6454..82b0d0cf0 100644
--- a/test/functional/attachments_controller_test.rb
+++ b/test/functional/attachments_controller_test.rb
@@ -223,12 +223,21 @@ class AttachmentsControllerTest < ActionController::TestCase
     set_tmp_attachments_directory
   end
 
-  def test_show_file_without_container_should_be_denied
+  def test_show_file_without_container_should_be_allowed_to_author
     set_tmp_attachments_directory
     attachment = Attachment.create!(:file => uploaded_test_file("testfile.txt", "text/plain"), :author_id => 2)
 
     @request.session[:user_id] = 2
     get :show, :id => attachment.id
+    assert_response 200
+  end
+
+  def test_show_file_without_container_should_be_allowed_to_author
+    set_tmp_attachments_directory
+    attachment = Attachment.create!(:file => uploaded_test_file("testfile.txt", "text/plain"), :author_id => 2)
+
+    @request.session[:user_id] = 3
+    get :show, :id => attachment.id
     assert_response 403
   end
 
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index e91fbd41c..6c0ea7fca 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -1000,7 +1000,7 @@ class IssuesControllerTest < ActionController::TestCase
     get :show, :id => 1
 
     assert_select 'form#issue-form[method=post][enctype=multipart/form-data]' do
-      assert_select 'input[type=file][name=?]', 'attachments[1][file]'
+      assert_select 'input[type=file][name=?]', 'attachments_files'
     end
   end
 
@@ -1569,8 +1569,7 @@ class IssuesControllerTest < ActionController::TestCase
     get :new, :project_id => 1, :tracker_id => 1
 
     assert_select 'form[id=issue-form][method=post][enctype=multipart/form-data]' do
-      assert_select 'input[name=?][type=file]', 'attachments[1][file]'
-      assert_select 'input[name=?][maxlength=255]', 'attachments[1][description]'
+      assert_select 'input[name=?][type=file]', 'attachments_files'
     end
   end
 
@@ -2165,7 +2164,7 @@ class IssuesControllerTest < ActionController::TestCase
     assert_nil attachment.container
 
     assert_tag 'input', :attributes => {:name => 'attachments[p0][token]', :value => attachment.token}
-    assert_tag 'span', :content => /testfile.txt/
+    assert_tag 'input', :attributes => {:name => 'attachments[p0][filename]', :value => 'testfile.txt'}
   end
 
   def test_post_create_with_failure_should_keep_saved_attachments
@@ -2184,7 +2183,7 @@ class IssuesControllerTest < ActionController::TestCase
     end
 
     assert_tag 'input', :attributes => {:name => 'attachments[p0][token]', :value => attachment.token}
-    assert_tag 'span', :content => /testfile.txt/
+    assert_tag 'input', :attributes => {:name => 'attachments[p0][filename]', :value => 'testfile.txt'}
   end
 
   def test_post_create_should_attach_saved_attachments
@@ -2967,7 +2966,7 @@ class IssuesControllerTest < ActionController::TestCase
     assert_nil attachment.container
 
     assert_tag 'input', :attributes => {:name => 'attachments[p0][token]', :value => attachment.token}
-    assert_tag 'span', :content => /testfile.txt/
+    assert_tag 'input', :attributes => {:name => 'attachments[p0][filename]', :value => 'testfile.txt'}
   end
 
   def test_put_update_with_failure_should_keep_saved_attachments
@@ -2986,7 +2985,7 @@ class IssuesControllerTest < ActionController::TestCase
     end
 
     assert_tag 'input', :attributes => {:name => 'attachments[p0][token]', :value => attachment.token}
-    assert_tag 'span', :content => /testfile.txt/
+    assert_tag 'input', :attributes => {:name => 'attachments[p0][filename]', :value => 'testfile.txt'}
   end
 
   def test_put_update_should_attach_saved_attachments
diff --git a/test/functional/issues_controller_transaction_test.rb b/test/functional/issues_controller_transaction_test.rb
index 6c662d89d..464307634 100644
--- a/test/functional/issues_controller_transaction_test.rb
+++ b/test/functional/issues_controller_transaction_test.rb
@@ -104,7 +104,7 @@ class IssuesControllerTransactionTest < ActionController::TestCase
     assert_template 'edit'
     attachment = Attachment.first(:order => 'id DESC')
     assert_tag 'input', :attributes => {:name => 'attachments[p0][token]', :value => attachment.token}
-    assert_tag 'span', :content => /testfile.txt/
+    assert_tag 'input', :attributes => {:name => 'attachments[p0][filename]', :value => 'testfile.txt'}
   end
 
   def test_update_stale_issue_without_notes_should_not_show_add_notes_option
diff --git a/test/integration/attachments_test.rb b/test/integration/attachments_test.rb
new file mode 100644
index 000000000..bd5a99439
--- /dev/null
+++ b/test/integration/attachments_test.rb
@@ -0,0 +1,132 @@
+# Redmine - project management software
+# Copyright (C) 2006-2012  Jean-Philippe Lang
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+require File.expand_path('../../test_helper', __FILE__)
+
+class AttachmentsTest < ActionController::IntegrationTest
+  fixtures :projects, :enabled_modules,
+           :users, :roles, :members, :member_roles,
+           :trackers, :projects_trackers,
+           :issue_statuses, :enumerations
+
+  def test_upload_as_js_and_attach_to_an_issue
+    log_user('jsmith', 'jsmith')
+
+    token = ajax_upload('myupload.txt', 'File content')
+
+    assert_difference 'Issue.count' do
+      post '/projects/ecookbook/issues', {
+          :issue => {:tracker_id => 1, :subject => 'Issue with upload'},
+          :attachments => {'1' => {:filename => 'myupload.txt', :description => 'My uploaded file', :token => token}}
+        }
+      assert_response 302
+    end
+
+    issue = Issue.order('id DESC').first
+    assert_equal 'Issue with upload', issue.subject
+    assert_equal 1, issue.attachments.count
+
+    attachment = issue.attachments.first
+    assert_equal 'myupload.txt', attachment.filename
+    assert_equal 'My uploaded file', attachment.description
+    assert_equal 'File content'.length, attachment.filesize
+  end
+
+  def test_upload_as_js_and_preview_as_inline_attachment
+    log_user('jsmith', 'jsmith')
+
+    token = ajax_upload('myupload.jpg', 'JPEG content')
+
+    post '/issues/preview/new/ecookbook', {
+        :issue => {:tracker_id => 1, :description => 'Inline upload: !myupload.jpg!'},
+        :attachments => {'1' => {:filename => 'myupload.jpg', :description => 'My uploaded file', :token => token}}
+      }
+    assert_response :success
+
+    attachment_path = response.body.match(%r{<img src="(/attachments/download/\d+)"})[1]
+    assert_not_nil token, "No attachment path found in response:\n#{response.body}"
+
+    get attachment_path
+    assert_response :success
+    assert_equal 'JPEG content', response.body
+  end
+
+  def test_upload_and_resubmit_after_validation_failure
+    log_user('jsmith', 'jsmith')
+
+    token = ajax_upload('myupload.txt', 'File content')
+
+    assert_no_difference 'Issue.count' do
+      post '/projects/ecookbook/issues', {
+          :issue => {:tracker_id => 1, :subject => ''},
+          :attachments => {'1' => {:filename => 'myupload.txt', :description => 'My uploaded file', :token => token}}
+        }
+      assert_response :success
+    end
+    assert_select 'input[type=hidden][name=?][value=?]', 'attachments[p0][token]', token
+    assert_select 'input[name=?][value=?]', 'attachments[p0][filename]', 'myupload.txt'
+    assert_select 'input[name=?][value=?]', 'attachments[p0][description]', 'My uploaded file'
+
+    assert_difference 'Issue.count' do
+      post '/projects/ecookbook/issues', {
+          :issue => {:tracker_id => 1, :subject => 'Issue with upload'},
+          :attachments => {'p0' => {:filename => 'myupload.txt', :description => 'My uploaded file', :token => token}}
+        }
+      assert_response 302
+    end
+
+    issue = Issue.order('id DESC').first
+    assert_equal 'Issue with upload', issue.subject
+    assert_equal 1, issue.attachments.count
+
+    attachment = issue.attachments.first
+    assert_equal 'myupload.txt', attachment.filename
+    assert_equal 'My uploaded file', attachment.description
+    assert_equal 'File content'.length, attachment.filesize
+  end
+
+  def test_upload_as_js_and_destroy
+    log_user('jsmith', 'jsmith')
+
+    token = ajax_upload('myupload.txt', 'File content')
+
+    attachment = Attachment.order('id DESC').first
+    attachment_path = "/attachments/#{attachment.id}.js?attachment_id=1"
+    assert_include "href: '#{attachment_path}'", response.body, "Path to attachment: #{attachment_path} not found in response:\n#{response.body}"
+
+    assert_difference 'Attachment.count', -1 do
+      delete attachment_path
+      assert_response :success
+    end
+
+    assert_include "$('#attachments_1').remove();", response.body
+  end
+
+  private
+
+  def ajax_upload(filename, content, attachment_id=1)
+    assert_difference 'Attachment.count' do
+      post "/uploads.js?attachment_id=#{attachment_id}&filename=#{filename}", content, {"CONTENT_TYPE" => 'application/octet-stream'}
+      assert_response :success
+      assert_equal 'text/javascript', response.content_type
+    end
+
+    token = response.body.match(/\.val\('(\d+\.[0-9a-f]+)'\)/)[1]
+    assert_not_nil token, "No upload token found in response:\n#{response.body}"
+    token
+  end
+end