[D7] upload_progress

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxwmnnd2402133git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Hi wmnnd,

Please fix the README file, lines should not exceed 80 characters. Also please use this template:
https://www.drupal.org/node/2181737

Instead of adding the jQuery form file from your module module why don't you rely on jquery_update and require a specific version.

There is an old module named upload_progress. I think you need another name for the module.

On a node/add form that has a file upload field I get the following error triggered from your js file. So I was unable to test how the module works.

SyntaxError: missing : after property id
function bytes2string(bytes) {

I've updated the git clone command.

wmnnd,

jQuery Update module will add the jquery.form.min.js?v=2.69. Isn't that sufficient?

Other than that, the jQuery Form Plugin you add requires jQuery v1.5 or later, so you don't you need the jquery update module anyway?
There's an issue regarding updating the jquery.form lib trough jQuery Update module here https://www.drupal.org/node/2266303

Also, as per https://www.drupal.org/node/1587704 4. Licensing checks

The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.

Hi Wmnnd,

You have some error on automated review:

FILE: /var/www/drupal-7-pareview/pareview_temp/js/jquery.form.min.js
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
1 | WARNING | File appears to be minified and cannot be processed
--------------------------------------------------------------------------------

Please use hook_libraries_info() to fix this.

Any further progress here? This would be an extremely useful module for the community as FCGI/Apache is very common.

@wmnnd when using this on a couple of themes the JS breaks using version jquery 1.10. That is my issue.

Hi wmnnd,
we run the PHP_CodeSniffer for your code here is one error

FILE: /var/www/drupal-7-pareview/pareview_temp/js/jquery.form.min.js
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
1 | WARNING | File appears to be minified and cannot be processed

----------------------------------------------------------------------

I suggest you follow these links to remove this error
As follows
https://github.com/squizlabs/PHP_CodeSniffer/issues/252
https://code.google.com/p/minify/wiki/CommonProblems
https://www.drupal.org/node/1556034

these links helps to remove this error
wmnnd, keep it up :)

Hi wmnnd,
we run the PHP_CodeSniffer for your code here is one error

FILE: /var/www/drupal-7-pareview/pareview_temp/js/jquery.form.min.js
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
1 | WARNING | File appears to be minified and cannot be processed

----------------------------------------------------------------------

I suggest you follow these links to remove this error
As follows
https://github.com/squizlabs/PHP_CodeSniffer/issues/252
https://code.google.com/p/minify/wiki/CommonProblems
https://www.drupal.org/node/1556034

these links helps to remove this error
wmnnd, keep it up :)

@prajaankit: removing review bonus, you should only add this to your own application. Please read https://www.drupal.org/node/1975228 again.

@wmnnd:

PAReview: 3rd party code

jquery.form.min.js appears to be 3rd party code. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms.

The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.

The problem is not licensing, our policy for contributed projects is simply:

In general 3rd party libraries are forbidden, so do not commit any. Instead, document for your users how to find and install the library themselves.

Just because some existing modules violate that policy does not mean that the policy is void.

Yes, there is discussion in the LWG currently. Meanwhile the existing policy stands and is an application blocker.

Manual Review

Individual user account

[Yes: Follows] the guidelines for individual user accounts.

No duplication

[No: Causes] module duplication and/or fragmentation.

Master Branch

[Yes: Follows] the guidelines for master branch.

Licensing

[Yes: Follows] the licensing requirements.

3rd party assets/code

[Yes: Follows] the guidelines for 3rd party assets/code.

README.txt/README.md

[Yes: Follows] the the guidelines for in-project documentation.

Code long/complex enough for review

[Yes: Follows] the guidelines for project length and complexity.

Secure code

I haven't done any security review

@wmnnd: Could it be a part of FileField Nginx Progress module. As you already mentioned the detail about filefield_nginx_progress module.

I think this module is very valid, I need to test it again as it was breaking my JS using BS.

Hi, if the scenario you are trying to solve is .../file/add then using file_entity, multiform and plupload provides a nice multiple file upload widget with a progress bar, drag and drop capability, does not require the PECL uploadprogress library or APC and it also works with fastcgi/apache. So, this might be considered duplication.

The file_entity module does not cover the case of adding an image to content though, that still appears as the standard progress bar which doesn't work in my environment.

I wanted to install and test but I'm having trouble getting it to work. I have the libraries and upload_progress modules enabled. I have the sites/all/libraries/jquery_form folder with files, but still just getting my plain old (non-working) progress bar. The js files are retrieved by the browser. Is there anything else I can check?

Automated Review

Passes all tests.

Manual Review

It took me a while to realise that without either the uploadprogress PECL module or APC installed there's no option on the file field widget to display a progress bar instead of a throbber. At a minimum the documentation should be changed to note this, although I think ideally the file field widget settings form should be altered to allow access to this setting even where file_progress_implementation() returns false.

Generally though, I think this is a fairly well written module that does one thing and does it well. Subject to this change plus adding a note about alternative modules and strategies for achieving a progress bar (see below) I'd be happy to see this as a full module. I have at least one site where I could make use of this module myself.

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

No: Causes module duplication and/or fragmentation. As well as those modules listed above, modules such as https://www.drupal.org/project/fine_uploader, https://www.drupal.org/project/dragndrop_upload and https://www.drupal.org/project/dragndrop_uploads all seem to provide an upload progress bar using XMLHttpRequest.upload. This is not considered an application blocker, however reference to these other modules and alternative methods of achieving progress bars should be made on the project page.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity. The total source code is less than 120 lines long, but does define more than 5 functions (including the javascript).

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

1. (+) core = 7.x should be removed from the .info file; this is added by the drupal packaging system.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

This review uses the Project Application Review Template.

correction: "core = 7.x" must stay in the info file so that the module works correctly when cloned with git.

Looks like you found no other application blockers, so should this be set to RTBC instead?

Apologies, just discovered the same issue with leaving "core = 7.x" in the info file for my own project. This is confusing in the docs elsewhere (I'll try and fix / file a separate issue there, assuming I can re-find it.)

Anyway, agree nothing else is an application blocker per se, happy to set to RTBC.

Installed again and works super well. Let's get a stable release, very much needed and super handy!!

The only drama on my end is getting this to work with a theme like Bootstrap. Right now you just get an empty bar. This is on the latest version of Bootstrap and Drupal.

Any thoughts as to what could be done to make this Bootstrap compatible?

@wmnnd. Although I set this to RTBC you still need a git administrator to grant you the Create Full Projects role before you can publish it. The git administrator is supposed to have a final quick scan through your code first. I did think it was supposed to happen automatically though after a few weeks at RTBC, but it seems not.

The best way to persuade a git administrator to look at your code is to review at least three other projects and list them in the issue summary after which you'll be entitled to add the 'PAReview: review bonus' tag. This is a good thing to do for its own sake, but I do also think though (given that this has been sat at RTBC for 4 months) it would be perfectly reasonable to raise the issue priority, or perhaps even to hit up a git administrator on IRC.

I found a pretty simple fix to make this work with bootstrap. Just replace this line in upload_progress.js :

 jQuery("input[type=submit]", ".file-widget, .image-widget, .video-widget").each(function(el) {

with

 jQuery("[type=submit]", ".file-widget, .image-widget, .video-widget").each(function(el) {

Basically, this is broken on Bootstrap because Bootstrap uses a button element, as opposed to an input. The modification above makes this work with any submit element (button or input).

Including a patch for the developers.

Patch does not work for me, same issue occurs, plus it causes an undefined error with the insert module. Trying to get more concrete info.

As the module with the same namespace is 8 years old and seeking new maintainers, i've added you to the maintainers of https://www.drupal.org/project/upload_progress so you can move your code there, or you can change your module's name.

Thanks for your contribution! Congratulations, you are now a vetted Git user. You can promote this to a full project.

When you create new projects (typically as a sandbox to start) you can then promote them to a full project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues (here you get to ask fans of your project to help you!)

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

If you want this to work on managed_file form elements then open upload_progress/js/upload_progress.js and change the CSS classes it targets to be something like this

jQuery("input[type=submit]", ".file-widget, .image-widget, .form-type-managed-file").each(function(el) {

instead of

jQuery("input[type=submit]", ".file-widget, .image-widget").each(function(el) {

Any chance backporting this for Durpal 6.x?

Hello, i would appreciate some help trying to backport this to drupal 6. So far i inserted the following javascript into a .js file loaded by my .info theme file. I am also running latest jquery.form.js as inticated in the readme.txt file. Thing is drupal 6 uses a ahah.js and a progress.js which are different than drupal 7. But in any case, the following code stops the get requests happening so far for the uploadprogress but it seems that there is a problem i am not aware off.

Drupal.behaviors.upload_progress = function (context, settings) {
    $("input[type=submit]", ".widget-edit").each(function(el) {
      var ahah = Drupal.settings.ahah[this.id];
      ahah.progress.url = undefined;
      ahah.uploadProgress = function(event, partial, total, percentage) {
         if (ahah.progress.object) {
          ahah.progress.object.setProgress(percentage, Drupal.t("Uploading... (current of total)", {
            current: Drupal.upload_progress.bytes2string(partial),
            total: Drupal.upload_progress.bytes2string(total)
          }));
        }
      }
    });
}

/**
 *  *  *  * Get human-readable string (e.g. 1.25 MB) from amount of bytes.
 *   *   *   */
Drupal.upload_progress = Drupal.upload_progress || {
  bytes2string: function (bytes) {
    var i = parseInt(Math.floor(Math.log(bytes) / Math.log(1000)));
    return (bytes / Math.pow(1000, i)).toPrecision(3) + " " + ["B", "KB", "MB", "GB"][i];
  }
}

It never enters ahah.uploadProgress.

Just thought i'd share the backporting to drupal 6.

I managed to get a working version with latest jquery.form.js

Here is the code i added in ahah.js

This is almost similar to the version in the module, although it has a more accurate file size calculation while the code stops the get requests fired for getting the statistics through APCu or uploadprogress PHP modules.

Drupal.behaviors.upload_progress = function (context) {
    $("input[type=submit]", ".widget-edit").each(function(el) {
      var ahah = Drupal.settings.ahah[this.id];
      ahah.progress.url = undefined;
    });
}

Drupal.upload_progress = Drupal.upload_progress || {
  bytes2string: function (a,b){if(0==a)return"0 Bytes";var c=1024,d=b||2,e=["Bytes","KB","MB","GB","TB","PB","EB","ZB","YB"],f=Math.floor(Math.log(a)/Math.log(c));return parseFloat((a/Math.pow(c,f)).toFixed(d))+" "+e[f]}
}

An addition to the uploadProgress prototype that exposes the XMLHttpRequest through jquery.form.js

/**
 * Processes file upload progress if needed.
 * @param event
 * @param position
 * @param total
 * @param percentComplete
 */
Drupal.ahah.prototype.uploadProgress = function (event, position, total, percentComplete) {
  if (this.progress.object) {
   this.progress.object.setProgress(percentComplete, Drupal.t("Uploading... (current of total)", {
       current: Drupal.upload_progress.bytes2string(position),
       total: Drupal.upload_progress.bytes2string(total)
   }));
 }
};

And finaly the function after the beforeSubmit and prior to the success events within the ahah options variable.

uploadProgress: function(event, position, total, percentComplete) {
       ahah.uploadProgress(event, position, total, percentComplete);
    },

All the parts go into ahah.js.Tested it with mozilla, safari, opera, ieEdge, chrome and works beautifuly. Just make sure you have the latest jquery.form.js loaded. Don't forget to add a translation for other languages for "Uploading... (current of total)" string. Cheers.