Problem/Motivation

"Bar with progress meter" in field is not show during upload files

Reproduce:
1. enable "Bar with progress meter" in fields
2. upload a file.

Proposed resolution

Getting it show normally.
(http://drupal.org/files/issues/screenshot_after_patched.jpg)

Remaining tasks

1. Needs reviews patch #70
- test it with PECL uploadprogress (#93, #96 tested & passed)
- test it with APC (#99, #100, if you hit Err 500, try to upgrade your APC to stable version (3.1.9+)
- needs code review (#93 reviewed)

2. Write better code comments (see #103 suggestion)

(CLEAN CACHES AFTER PATCHED YOUR SITE)

User interface changes

(http://drupal.org/files/issues/screenshot_after_patched.jpg)

API changes

--

Original report by [Tom Jacobs]

If I set to "Bar with progress meter" in field files. I then upload heavy file. results show no bar with progress meter.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Tom Jacobs’s picture

Title: Bar with progress meter by "PECL uploadprogress" (php) » show no: Bar with progress meter by "PECL uploadprogress" (php)
Version: 7.0-beta1 » 7.x-dev
adrinux’s picture

Title: show no: Bar with progress meter by "PECL uploadprogress" (php) » PECL uploadprogress bar doesn't appear.
Version: 7.x-dev » 7.0-rc1

I'm seeing this too, on two different servers running Ubuntu 10.04.1 LTS, on both servers PECL uploadprogress bar is working perfectly in Drupal-6, just not in Drupal-7.

Add an image field to a custom content-type and in 'manage fields' set progress indicator to 'Bar with progress meter'. (public files, single value)

From status report:
PHP 5.3.2-1ubuntu4.5
Database MySQL 5.1.41-3ubuntu12.7
Upload progress Enabled (PECL uploadprogress)
Web server Apache/2.2.14 (Ubuntu)

No apparent errors to explain why.

adrinux’s picture

I will add that the progress bar doesn't appear in Safari, Opera or Firefox.

assert0’s picture

I see this problem as well with PECL uploadprogress properly installed. I am only seeing the throbber when uploading, even when I've selected "Bar with progress meter" as the "Progress Indicator".

From status report:
drupal 7.0-rc2
PHP 5.3.3-1ubuntu9.1
Upload progress Enabled (PECL uploadprogress) OK
Web server Apache/2.2.16 (Ubuntu)

indigoblue’s picture

#4 ditto

D7-RC2
Centos 2.6 on x86_64
php 5.2.14
Apache 2.2.3
Upload progress Enabled (PECL uploadprogress) OK

droplet’s picture

Version: 7.0-rc1 » 7.x-dev

bump up to Dev.

7.0-dev
5.1.37-1ubuntu5.5
Enabled (APC RFC1867)
Apache/2.2.12 (Ubuntu)

sobi3ch’s picture

Check your Suhosin patch version..

Problems with Suhosin

The suhosin extension had a bug which prevented the uploadprogress from working properly. This bug was fixed in Suhosin 0.9.26 last August.

http://blog.liip.ch/archive/2009/03/31/upload-progress-meter-common-issu...
adrinux’s picture

Thanks for the heads up @sobi3ch

FWIW my Suhosin Patch 0.9.9.1, default Ubuntu 10.04.1 LTS package.

droplet’s picture

Component: file system » field system

settings always return throbber

"progress":{"type":"throbber","message":null
assert0’s picture

FileSize
61.49 KB

I confirm #9.. throbber is always set. I haven't been able to track down why the setting is not getting set, so I tried manually setting the #progress_indicator to 'bar' in the code segment below, however now I get an ajax error shown in the attached image.

--- a/modules/file/file.module
+++ b/modules/file/file.module
@@ -70,7 +70,7 @@ function file_element_info() {
     '#pre_render' => array('file_managed_file_pre_render'),
     '#theme' => 'file_managed_file',
     '#theme_wrappers' => array('form_element'),
-    '#progress_indicator' => 'throbber',
+    '#progress_indicator' => 'bar',
     '#progress_message' => NULL,
     '#upload_validators' => array(),
     '#upload_location' => NULL,
droplet’s picture

I did some trace
1. always throbber
2. return invalid json ( first output from file_ajax_progress(), and the delivery callback "ajax_deliver" added ajax_render() outputs )

{"message":"Starting upload...","percentage":-1}[{"command":"settings","settings":{"basePath":"\/drupal7x\/","ajaxPageState":{"theme":"bartik","theme_token":"4tgJdH7t7H4xpVRBDIlWbSGk98Xvz5fqZCnrY5MxPeo","css":[]}},"merge":true}]
droplet’s picture

Priority: Normal » Major
yched’s picture

Component: field system » file.module

Recategorizing

assert0’s picture

One of the issues is that the UPLOAD_IDENTIFIER field is showing up after the "file" type in the form. The blog post mentioned in #7 (http://blog.liip.ch/archive/2009/03/31/upload-progress-meter-common-issu...) states the following..

The UPLOAD_IDENTIFIER field in your upload form is very important. Without this field, it won't work properly. Furthermore, the field has to be before the file upload fields in the form. The way it works on the server side is asking for that order.

To fix this you need to add a weight to the UPLOAD_IDENTIFIER field to push it up to the top of the form as done with the code below.

--- a/modules/file/file.module
+++ b/modules/file/file.module
@@ -409,6 +409,7 @@ function file_managed_file_process($element, &$form_state, $form) {
         '#type' => 'hidden',
         '#value' => $upload_progress_key,
         '#attributes' => array('class' => array('file-progress')),
+        '#weight' => -20,
       );
     }
     elseif ($implementation == 'apc') {

This fix at least allows us to call the uploadprogress_get_info function and get the status of our upload; however, this still doen't fix the fact that the setting is always throbber and the fact that we are getting a second call to the drupal_json_output function which creates an invalid json object.

ogi’s picture

subscribe

ogi’s picture

In addition to changing the hardcoded #progress_indicator to bar and setting #weight, I commented out the 'delivery callback' and 'theme callback' entries for 'file/progress' in file_menu(). This is not a complete solution because hardcoded progress indicator still needs to be fixed.

ogi’s picture

Oh, I forgot to say that this way the progress indicator is shown :-)

Anonymous’s picture

Subscribe. Note: v. 7.0 exhibits the same behavior, so this isn't something that got broken in the .dev branch.

droplet’s picture

Status: Needs review » Needs work
FileSize
1.96 KB

Temporary solve my problem, I think there have a more right, simple & drupal way to do that.
(don't forget clean caches when you use this patch)

apaderno’s picture

Status: Active » Needs review
assert0’s picture

Status: Needs work » Needs review
FileSize
2.25 KB

Patch in #19 works great for me. Unfortunately the progress meter is still missing. To fix that, the file.css file needs an update. This patch adds the css fix to the patch in #19.

assert0’s picture

FileSize
2.23 KB

sorry rolled up the patch wrong.. use this patch instead.

rfay’s picture

Status: Needs review » Needs work

Just putting this in "needs work" because the testbots seem to be getting in an infinite loop of some type. Ok to retest later.

calte’s picture

Any movement on this?

stoptime’s picture

Subscribe. Same problem in 7.0

droplet’s picture

Status: Needs work » Needs review

#22: 935208_3.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 935208_3.patch, failed testing.

nilsja’s picture

subscribing. also dont get the "with progress meter" option running...

jarizaro’s picture

FileSize
20.28 KB

For me... patch #22 is working only at half,

i see the progress bar metter, and "Starting upload..." until download finish. BUT i only see a bit of blue. and progress bar doesn't move. Upload file is ok.

Tested in google chrome and firefox.

jarizaro’s picture

no working solution?

steinmb’s picture

Broken om my D7 site, works just fine on my D6-sites on the same server.
Drupal 7.0
PHP 5.2.6 with APC cache enabled
Upload Progress ver. 1.0.1

The #22 patch just make i worse then it throw this error when upload starts:

An AJAX HTTP error occurred. HTTP Result Code: 200 Debugging information follows. Path: /file/progress/2119719244 StatusText: OK ResponseText: {"message":"Starting upload...","percentage":-1}[{"command":"settings","settings":{"basePath":"\/","ajaxPageState":{"theme":"bartik","theme_token":"feaPnRIlk0Q-GR3kAmtaUOr6k_GU4gebNw8XeNaGuTM","css":[]},"colorbox":{"transition":"elastic","speed":350,"opacity":"0.85","slideshow":false,"slideshowAuto":false,"slideshowSpeed":2500,"slideshowStart":"","slideshowStop":"","current":"{current} of {total}","previous":"\u00ab Prev","next":"Next \u00bb","close":"Close","overlayClose":true,"maxWidth":"100%","maxHeight":"100%","__drupal_alter_by_ref":["default"]},"overlay":{"paths":{"admin":"overlay\/dismiss-message\nadmin\nadmin\/*\nbatch\ntaxonomy\/term\/*\/edit\nuser\/*\/cancel\nuser\/*\/edit\nuser\/*\/edit\/*","non_admin":"admin\/structure\/block\/demo\/*\nadmin\/reports\/status\/php"},"ajaxCallback":"overlay-ajax"}},"merge":true}]

mariomaric’s picture

Version: 7.x-dev » 8.x-dev
Issue tags: +Needs backport to D7

As far as I understand, this bug first needs to be fixed in D8, and then backported, hence I changed the version info and added the needs backport to D7 tag.

More info about the backport policy.

Ah, yes, also subscribing. ;)

Tor Arne Thune’s picture

Status: Needs work » Needs review
Issue tags: -Needs backport to D7

#22: 935208_3.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs backport to D7

The last submitted patch, 935208_3.patch, failed testing.

3rdLOF’s picture

subscribing.

Daemon_Byte’s picture

subscribing

munkyonline’s picture

subscribing

droplet’s picture

Assigned: Unassigned » droplet
Status: Needs work » Needs review
FileSize
2.32 KB

Try this one, more perfect now :)

** CLEAR CACHES AFTER PATCHED IT **

3rdLOF’s picture

A) Patch worked.
B) After clearing patched, it worked, but only partially

In my case before this patch nothing showed up. Now I can see the actual bar field shows up, but even when using the largest file I could find, the actual moving bar only showed up once and never with the standard blue/bars theme, just gray. I am testing on a local machine, but the bar always showed up in d6, even in local test sites.

I did and odd ajax error once. Never happened again.

3rdLOF’s picture

Switched over to Seven....So the blue bar is there but never actually moves pass the initial stage, so it is not the theme I am using.

Also, I have gotten this ajax error at least twice:

An AJAX HTTP request terminated abnormally.
Debugging information follows.
Path: /file/ajax/field_image_content/und/0/form-PTahDBBP9gL3C04puYfl3kf9cjrVXmaXb1TK8nF3I9g
StatusText: n/a
ResponseText: 
Error  
Error message
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'public://poster.jpg' for key 'uri': INSERT INTO {file_managed} (uid, filename, uri, filemime, filesize, status, timestamp) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6); Array
(
[:db_insert_placeholder_0] => 1
[:db_insert_placeholder_1] => poster.jpg
[:db_insert_placeholder_2] => public://poster.jpg
[:db_insert_placeholder_3] => image/jpeg
[:db_insert_placeholder_4] => 11974277
[:db_insert_placeholder_5] => 0
[:db_insert_placeholder_6] => 1304074839
)
in drupal_write_record() (line 6846 of /Users/luislopez/Sites/drupal7/includes/common.inc).
The website encountered an unexpected error. Please try again later.    

ReadyState: undefined

I suppose this is not related, but just in case it matters.

droplet’s picture

@kannary100,

yes, #40 is not related.

do you use APC or uploadprogress ?? I suppose they are same but I have only tested with APC.
try to open your browsers console ( firebug, chrome console) see what happening

EDIT: confirmed both APC or uploadprogress works. if you are not, please post more info ( APC / uploadprogress version, etc)

steinmb’s picture

YAY! #38 is working perfect. Fixes the problems I had with #22. I have not have not looked a lot at the patch but it is small, but important patch from a usability point of view. RTBC?

droplet’s picture

some more, I tested with nginx progress and sent a patch to that project : #1142338: Progress bar error in D7

sun’s picture

Status: Needs review » Needs work
+++ b/modules/file/file.module
@@ -45,7 +45,6 @@ function file_menu() {
   $items['file/progress'] = array(
     'page callback' => 'file_ajax_progress',
-    'delivery callback' => 'ajax_deliver',

Why is the delivery callback removed?

+++ b/modules/file/file.module
@@ -416,6 +416,7 @@ function file_managed_file_process($element, &$form_state, $form) {
         '#attributes' => array('class' => array('file-progress')),
+         '#weight' => -20,

Wrong indentation here.

Powered by Dreditor.

droplet’s picture

Status: Needs work » Needs review
FileSize
2.32 KB

it's similar to batch, return JSON back and progress.js will handle it. any reason it should use delivery callback?

Status: Needs review » Needs work
Issue tags: -Needs backport to D7

The last submitted patch, 935208-uploadprogress-bar-fix.patch, failed testing.

droplet’s picture

Status: Needs work » Needs review
Issue tags: +Needs backport to D7
anglo’s picture

Where can the patch for D7 be obtained?

steinmb’s picture

@anglo see #45

xgmorkx’s picture

@steinmb: as far as I can see, this patch is for D8 and still has to be backported to D7?

droplet’s picture

@xgmorkx, anglo,

It's patch still works for both D7 & D8 version.

steinmb’s picture

droplet’s picture

@steinmb,

seems like not easy to fix on nginx, until now I don't see any quick way to change the PATH.

function file_field_widget_form(&$form, &$form_state, $field, $instance, $langcode, $items, $delta, $element) {
...
...
    '#process' => array_merge($element_info['#process'], array('file_field_widget_process')),
...
...
.
...

we need some way to alter file_field_widget_form function, any idea ??

steinmb’s picture

Sorry, have not worked on/with hook_field_widget_form() in D7. Let's hope perhaps @sun find time to share some of his knowledge on the field-API.

sun’s picture

Status: Needs review » Needs work
+++ b/modules/file/file.css
@@ -20,7 +20,7 @@
 .form-managed-file div.ajax-progress div {
-  display: inline;
+  display: block;
 }

Someone needs to test whether this change retains proper styling 1) with APC, and 2) without any upload extension.

+++ b/modules/file/file.field.inc
@@ -440,6 +440,8 @@ function file_field_widget_settings_form($field, $instance) {
+  $widget = $instance['widget'];
+  $settings = $widget['settings'];

We can directly access the settings in $instance, no need for these variables.

+++ b/modules/file/file.module
@@ -45,7 +45,6 @@ function file_menu() {
   $items['file/progress'] = array(
     'page callback' => 'file_ajax_progress',
-    'delivery callback' => 'ajax_deliver',
     'access arguments' => array('access content'),
     'theme callback' => 'ajax_base_page_theme',

Based on the menu router item definition, as well as code in file_field_widget_process() that's using #ajax, the original intention was to use the AJAX framework to ensure proper theming and page callback result delivery. Therefore, I think we should keep ajax_deliver as delivery callback.

In turn, the actual problem is that file_ajax_progress attempts to generate a JSON response manually instead of passing the result back to the delivery callback.

+++ b/modules/file/file.module
@@ -409,6 +408,7 @@ function file_managed_file_process($element, &$form_state, $form) {
+        '#weight' => -20,

@@ -416,6 +416,7 @@ function file_managed_file_process($element, &$form_state, $form) {
+        '#weight' => -20,

According to the issue comments above, this #weight change seems to be essential. Therefore, both lines should each have a comment above them explaining why they are required, so they won't get changed or removed in the future.

13 days to next Drupal core point release.

adrinux’s picture

Status: Needs work » Reviewed & tested by the community

Applied patch from #45 to drupal-7.2, fixes the issue for me.

sun’s picture

Status: Reviewed & tested by the community » Needs work
droplet’s picture

Assigned: droplet » Unassigned

@sun,
Thanks
I'm not very sure. In this case we don't need delivery theming and page callback result back to Drupal ?

seems like it also need to update the progress.js as well if it use the AJAX framework, is it possible to make some changes on progress.js in D7 directly ?

anyone could make a patch ? (next release point coming, I have no time recently.) (and don't forget update on batch API)

adrinux’s picture

I've followed up on some of @sun's comments, updated version of @droplet's patch attached.

+ added a comments about the field weight change
+ added back the delivery callback, but that gives the ajax error other people have reported above:

An AJAX HTTP error occurred. HTTP Result Code: 200 Debugging information follows. Path: /file/progress/1310428177 StatusText: OK ResponseText: {"message":"Starting upload...","percentage":-1}[{"command":"settings","settings":{"basePath":"\/","pathPrefix":"","ajaxPageState":{"theme":"bartik","theme_token":"u0g2-DlBg56DIm4fPG0pBhIvk_K_LyBajVh4RUqsksc","css":[]}},"merge":true}]

- commented out the delivery call back for now. The ajax error needs fixed in this patch too.

Attached current work in progress patch.

adrinux’s picture

Status: Needs work » Needs review
FileSize
2.04 KB

I think I've addressed the ajax error now. So compared to @droplet's patch in #45 the attached patch:

+ added comments about the field weight change as per @sun's suggestion in #55
+ gets a variable directly from $instance and removes the two new ones as per @sun's suggestion in #55
+ added back the delivery callback as per @sun's suggestion in #55
+ fixes file_ajax_progress as per @sun's suggestion in #55

Works for me on Ubuntu 10.04 LTS with pecl uploadprogress 1.0.1 stable both with and without apc, in 7.x, and Drupal-7.22.
What it needs now is more testing:
> in 8.x
> without any upload extension. (in both 7.x and 8.x)

droplet’s picture

Status: Needs review » Needs work

adrinux,

are you sure its working ? on my latest test, seems like it should fix progress.js to handle the bar updates.

adrinux’s picture

Status: Needs work » Needs review

@droplet I wouldn't have written 'works for me' in #60 if it didn't :P
Obviously the conditions I tested are limited, as described in #60

Are you testing the reworked patch from #60?
Under what conditions (drupal branch etc)?
What breakage do you see?

droplet’s picture

yes. no error. but no progress updates :(

progress.js:

success: function (progress) {
// Display errors.
if (progress.status == 0) {
pb.displayError(progress.data);
return;
}
// Update display.
pb.setProgress(progress.percentage, progress.message);
// Schedule next timer.
pb.timer = setTimeout(function () { pb.sendPing(); }, pb.delay);
}, 

ajax response:

[{"command":"settings","settings":{"basePath":"\/drupal7x\/","pathPrefix":"","ajaxPageState":{"theme":"bartik","theme_token":"rPiMzGoTnIcaJMN2-PO9GmA-LO8Y5hLej60M8YwVbdM","css":[]}},"merge":true},{"command":"insert","method":"replaceWith","selector":null,"data":{"message":"Starting upload...","percentage":-1},"settings":{"basePath":"\/drupal7x\/","pathPrefix":"","ajaxPageState":{"theme":"bartik","theme_token":"rPiMzGoTnIcaJMN2-PO9GmA-LO8Y5hLej60M8YwVbdM"}}}]

new response is in an array?

do you cleared caches ??

** im tested on both D8/D7 master

adrinux’s picture

Status: Needs review » Needs work

I see, so how to fix it? I don't even see when progress.js kicks in? Is it only used if an uploadprogress extension isn't installed? Hard to test until you know how to break it :/
(no time left for this today…)

It always was an array, no? Maybe you can play with line 314 of file.module:
return array('#type' => 'ajax', '#commands' => $progress);
and come up with something that works for progress.js as well as uploadprogress, unless progress.js needs adapted to this change…

I should probably mention that I've only tested using Seven admin theme, not bartik.

sun’s picture

Tried to look into this, but running out of time.

Looks like I've been wrong. #ajax resp. Drupal.ajax.prototype.beforeSend in ajax.js calls into Drupal.progressBar of progress.js, and that expects a JSON response, so file_ajax_progress() is likely correct. What a mess.

However, file_ajax_progress() should properly end the request. It's a regular page callback, so Drupal attempts to render a full page afterwards. That was most probably the reason for specifying 'ajax_deliver' as "fake" delivery callback. By printing the JSON and simply returning nothing (NULL) from the page callback, ajax_deliver() does basically nothing, too. However, that's quite a hack.

Since the result has been printed already and that way can't be altered anyway, the drupal_json_output() should be followed by a drupal_exit(); -- when doing so, we can remove the delivery callback from the menu router item definition.

droplet’s picture

Status: Needs work » Needs review
FileSize
39.13 KB
2.33 KB

Well. we don't need set CSS to inline or block :)

aspilicious’s picture

Someone should test this in IE.

droplet’s picture

Assigned: Unassigned » droplet

Yes, tested. and need someone to do test too :)

It should be safe to remove :)

<div class="ajax-progress ajax-progress-throbber"><div class="throbber">&nbsp;</div></div>
.form-managed-file div.ajax-progress,
.form-managed-file div.throbber {
  display: inline; //defined
  float: none;
  padding: 1px 5px 2px 5px;
}

.form-managed-file div.ajax-progress div {
  display: inline; // redefine again
}

WOW, drupal looks very ugly in IE7 !!! Seems like Drupal community hate IE so much.

sun’s picture

Status: Needs review » Needs work
+++ b/modules/file/file.module
@@ -45,7 +45,6 @@ function file_menu() {
   $items['file/progress'] = array(
     'page callback' => 'file_ajax_progress',
-    'delivery callback' => 'ajax_deliver',

@@ -312,6 +311,7 @@ function file_ajax_progress($key) {
   drupal_json_output($progress);
+  drupal_exit();

ok, I'm terribly sorry having hi-jacked this issue. @pwolanin recently reminded my in another issue that returning NULL (or just simply not returning anything) from a page callback is sufficient, since drupal_deliver_html_page() will only call drupal_page_footer() in that case, which is functionally identical to drupal_exit().

Removing ajax_deliver() as delivery callback is a required part of the fix here, since the difference to drupal_deliver_html_page() is that ajax_deliver() always attempts to add JS/CSS file information for AJAX lazy-loading. However, this page callback is NOT invoked from ajax.js - it is invoked from progress.js, and therefore, the response has to be JSON, as expected by progress.js.

4 days to next Drupal core point release.

droplet’s picture

Status: Needs work » Needs review
FileSize
2.7 KB

it's a new update, we never use progress Bar on remove buttons.

PI_Ron’s picture

subscribe

PI_Ron’s picture

Patch in #66 worked for me, D7. patch in #70 produced showed the bar briefly, then pumped out an ajax error.

droplet’s picture

@Tigeda,

what's the error? can you give more info. and do you use APC or upload progress extension

Tsubo’s picture

Version: 8.x-dev » 7.x-dev
Status: Needs review » Needs work

On a centos box running drupal 7.x-dev version,
I experience a client-side javascript error on Firefox (version 3.6, 4, and 5 - all on OSx)

Patches obtained from the list through fix-67.patch have been applied.

The progress bar appears, and updates till about 75% full then stalls,
the upload completes but the browser then serves a json commands
array and the progress bar remains on screen.

This behaviour is not observed under safari (any version).

Essentially I belive this to be an error in the "upload_completed"
callback or what ever the drupal name is for the pertinant callback.

It is likely that the bug resides in ajax.js however further
investigation is required to narrow down the issue.

I suspect this issue also happens with the throbber, but doesn't
manifiest in the same way.

Hope this helps...

aspilicious’s picture

Version: 7.x-dev » 8.x-dev

Please leave this at 8.x as we fix bugs first in 8.x and backport them as fast as possible.

infines’s picture

subscribe

flokli’s picture

subscribe

infines’s picture

Priority: Major » Critical
aspilicious’s picture

Priority: Critical » Major

WHY should this be critical o_O

droplet’s picture

Status: Needs work » Needs review

@Tsubo #74,

IMO, this issue will only fix uploadprogress bar to make it show normally (and upload normally). Other error we can fix in a separated issue.

Needs code reviews

infines’s picture

My opinion, it should be critical, because its a feature that's apart of core. Drupal 7 has been released for 8 months now. I realize that there are other critical bugs and security issues, but some of the features that are supposed to work in a released product don't. Which isn't good for PR for a product. Handle it as you will though.

aspilicious’s picture

Critical bugs ar ebugs that make the site unusable.

Tsubo’s picture

any movement on this?

catch’s picture

@gridbitlabs "but some of the features that are supposed to work in a released product don't"

That's the case for every bug in core, why bother with statuses at all then? You haven't indicated whether you tested the patch in #70 or not. If people don't test the patches and report back, then the bug won't get fixed. Instructions are at http://drupal.org/patch/apply

chx’s picture

I think I grok #70 (although the issue summary could use some work -- it only explains what's visible of the error not the causes and the proposed fix merely points to #70 instead of explaining what it does) but

-.form-managed-file div.ajax-progress div {
-  display: inline;
-}

what's that? Why?

droplet’s picture

@chx,

#68 for more details

dlumberg’s picture

#70 In chrome with APC and uploadprogress installed.

It looks like it's sending back the AJAX and then it's loading jquery options that are completely unnecessary for the progress bar.

An AJAX HTTP error occurred. HTTP Result Code: 200 Debugging information follows. Path: /file/progress/2066878555 StatusText: OK ResponseText: {"message":"Starting upload...","percentage":-1}[{"command":"settings","settings":{"basePath":"\/","pathPrefix":"","ajaxPageState":{"theme":"zentheme","theme_token":"--bOmPL9bR2XgNAHQZ0jE-f5CmIlUb7fYB5EwjxwSbs","css":[]},"colorbox":{"opacity":"0.85","current":"{current} of {total}","previous":"\u00ab Prev","next":"Next \u00bb","close":"Close","maxWidth":"100%","maxHeight":"100%","__drupal_alter_by_ref":["default"]},"fancybox":{"options":{"margin":0,"padding":0,"speedIn":0,"speedOut":0,"changeSpeed":0},"selector":".fancybox"},"jcarousel":{"ajaxPath":"\/jcarousel\/ajax\/views"},"nice_menus_options":{"delay":"200","speed":"fast"},"overlay":{"paths":{"admin":"media\/*\/edit\nmedia\/*\/multiedit\nmedia\/*\/delete\nnode\/*\/group\ngroup\/*\/*\/admin\/*\nuser\/*\/openid\nuser\/*\/openid\/delete\noverlay\/dismiss-message\nuser\/*\/shortcuts\nadmin\nadmin\/*\nbatch\ntaxonomy\/term\/*\/edit\nuser\/*\/cancel\nuser\/*\/edit\nuser\/*\/edit\/*\ndevel\/*\nnode\/*\/devel\nnode\/*\/devel\/*\ncomment\/*\/devel\ncomment\/*\/devel\/*\nuser\/*\/devel\nuser\/*\/devel\/*\ntaxonomy\/term\/*\/devel\ntaxonomy\/term\/*\/devel\/*","non_admin":"admin\/structure\/block\/demo\/*\nadmin\/reports\/status\/php\nuser\/*\/edit\/twitter"},"ajaxCallback":"overlay-ajax"},"admin_menu":{"destination":"destination=file\/progress\/2066878555","hash":"92acf18da60a2f67c9be27b1d2fb84c1","basePath":"\/admin_menu","replacements":{".admin-menu-users a":"0 \/ 1"},"margin_top":1,"toolbar":[]}},"merge":true}]
sun’s picture

@dlumberg: Sounds like you didn't flush all caches after applying #70.

chx’s picture

#68 is "Yes, tested. and need someone to do test too :) It should be safe to remove :) " folowed by the stuff removed. Why it is safe to remove, why it was there in the first place etc?

droplet’s picture

@chx,

Okay. to be clearly..actually it's a BUG. Remove it to make it working normally.try to test the patch with/without it, you will see the different.

throbber = the icon will show next to the upload input
progress bar = show on a new line (http://drupal.org/files/issues/screenshot_after_patched.jpg) (in other word, it should be a block element or display: block)

here is the throbber html:

<div class="ajax-progress ajax-progress-throbber"><div class="throbber">&nbsp;</div></div>

and progress bar html:

<div aria-live="polite" class="progress ajax-progress ajax-progress-bar" id="ajax-progress-edit-field-test-und-0-upload-button" style="display: block;"><div class="bar"><div class="filled" style="width: 51%;"></div></div><div class="percentage">51%</div><div class="message">Starting upload...</div></div>

If you defined inner DIV with display:inline, there is no width of that elements.

throbber style has defined in

.form-managed-file div.ajax-progress,
.form-managed-file div.throbber {
  display: inline;
  float: none;
  padding: 1px 5px 2px 5px;
}

and following code is redefined it again (or you can think is a mistake):

.form-managed-file div.ajax-progress div {
  display: inline;
}

remove the code to get progress bar show normally and still don't affect throbber style.

mshick’s picture

I installed and tested the patch in #70 after having the same problem, and it solved everything after a cache clear.

I am running Ubuntu 10.04 LTS, and the APC upload progress library.

Using the Seven library I didn't notice any display issues.

Thanks!

Jazz Li’s picture

Subscribing.

chx’s picture

Status: Needs review » Reviewed & tested by the community

A number of bugs are fixed in here, if I understand correctly:

  1. Frontend.The inner div of the progress bar HTML is defined to display as inline and so there is no width of the progress bar.
  2. Invalid AJAX returned due to the unnecessary AJAX deliver
  3. Uploadprogress extension requires the hidden progressbar field to be at the top of the form.
  4. Remove button should not use a progress bar, only the upload button.

Given the positive reports I am RTBC'ing this.

steinmb’s picture

Status: Reviewed & tested by the community » Needs work

Unable to get #70 working. It only show the throbber but does not display any error messages. The patch I testet at #42 used to work, though have not tested it on D7.7 and yes the field setting is set to "Bar with progress meter"

Test environment:
http://pecl.php.net/package/uploadprogress ver. 1.0.1
PHP 5.2.6
Drupal 7.7
APC-3.1.9
Restartet Apache (to flush APC upcode cache) and a drush cc all.

droplet’s picture

@steinmb,
can you test it again ?? #70 and patch before #42 almost same.
have you refresh your browser pages ?

try to drush cc all few time :)

My env is same to you but im PHP 5.3

steinmb’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
39.77 KB

Ah, now it behaved correctly when adding and removing files. Sorry about that, let's get this one in :)

droplet’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
2.35 KB

Anyway thanks for info.

I find a problem on APC.
APC working in opposite direction,
if #weight=-20 on APC, click the button directly (no select any file) will get errors.

droplet’s picture

Anyone have chance to turn off PECL uploadprogress and do a test on APC ?

droplet’s picture

Issue summary: View changes

add issue summary

steinmb’s picture

PHP 5.2.6
Drupal 7.7
APC-3.1.9
uploadprogress.so disabled
APC and #97.

#97 break the progress bar on my server. It load the bar OK, but it is not showing (updating) the progress during upload. Going back to #70, and APC is performing correctly again. Not 100% on how to provoke the problem you are referring in #97. We are suppose to set the weight of the file-field to -20, and then click upload with no files selected? If so, did I try that, but without seeing any errors with #70 applied.

droplet’s picture

Status: Needs review » Reviewed & tested by the community

Okay.

upgrade my APC form beta to 3.1.9 stable version solved my problem.

Back to RTBC #70 is the right patch

droplet’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/modules/file/file.module
@@ -381,7 +380,8 @@ function file_managed_file_process($element, &$form_state, $form) {
-  $ajax_settings['progress']['type'] ? $ajax_settings['progress']['type'] == 'bar' : 'throbber';
+  // Set up the remove button use throbber or none.
+  $ajax_settings['progress']['type'] = ($element['#progress_indicator'] == 'none') ? 'none' : 'throbber';

This "resets" everything to "none" or "throbber". What happened to "bar"?

Perhaps we want to make it a follow-up issue, but AFAIK, the idea here was to make the progress implementation 'pluggable', so if I wanted to, I could have my own custom "beerbar" implementation.

If the code purposively intends and prevents me from using my implementation, then we need a better inline comment explaining why that is done.

15 days to next Drupal core point release.

droplet’s picture

Removing an upload is less than 1 sec, show a bar is no meaning I think. (and there isn't a way to tracking remove action metre ?)

It still able to do custom beerbar implementation.

Steven Jones’s picture

+++ b/modules/file/file.module
@@ -381,7 +380,8 @@ function file_managed_file_process($element, &$form_state, $form) {
-  $ajax_settings['progress']['type'] ? $ajax_settings['progress']['type'] == 'bar' : 'throbber';
+  // Set up the remove button use throbber or none.
+  $ajax_settings['progress']['type'] = ($element['#progress_indicator'] == 'none') ? 'none' : 'throbber';

@sun This code is only concerned with the ajax settings on the remove button at this point, which we're restricting to only use either 'none' or 'throbber', which is the only two that probably make sense for the remove button anyway. You can still have a custom progress implementation on the main file upload itself.

Agreed that the comment on this code could and should be better though.

How about:

// Force the progress indicator for the remove button to be either 'none' or
// 'throbber', even if the upload button is using something else.

12 days to next Drupal core point release.

dddbbb’s picture

sub

dddbbb’s picture

#70 & #97 just resulted in great big AJAX errors. #66 worked for me kind of (I'm using PECL uploadprogress) but I still get a progress bar on clicking remove. On one of my servers the progress bar never goes away on remove (not good) on another server it appears on remove but eventually goes away and the file is removed.

Also, I'm getting a differently styled progress bar on each server - anyone have any ideas what could be causing that?

droplet’s picture

Apply #70 and clean caches.

Post your sever info if no helps.

dddbbb’s picture

That sorted it, nice one. Turns out my odd styling issue was entirely cache related also.

dddbbb’s picture

Issue summary: View changes

Updated issue summary.

Nico Heulsen’s picture

subscribe

bluestarstudios’s picture

So is patch from #70 the one we should use right now? Will it work in Drupal 7? Thanks.

droplet’s picture

Yes. working in D7

infines’s picture

This should be committed before tomorrow!

droplet’s picture

is suggestion on #103 good enough ? I will reroll it.

droplet’s picture

Issue summary: View changes

Updated issue summary.

Nico Heulsen’s picture

I can confirm that patch #70 is working in combination with the PECL uploadprogress version 1.0.1. The version of the PECL extension is important. With the latest version (1.0.3.1) it will show the progressbar, but no progress.

jonesmac’s picture

I patched Drupal 7.8 and had no luck getting it to even appear. I cleared my cache twice. I applied patch #70. Anyone have and thoughts as to what else to try?

Nico Heulsen’s picture

@jonesmac What message do you get on the status report? I think you also have to change a setting of APC in your server configuration.

jonesmac’s picture

Under Upload Progress, it simply has it as "Enabled (PECL uploadprogress)".

I applied patch #70 and nothing changed.

I have this is my log:

"Notice: Undefined index: progress_indicator in file_field_widget_form() (line 485 of /home/mydirectory/file/file.field.inc)."

Not sure if its from the patch or not. I am running drupal 7.8

heshanlk’s picture

sub

jonesmac’s picture

Disregard #114. It appears to be an issue with the audiofield module. For some reason it doesn't allow you to select between progress bar and throbber when uploading audio files. Don't believe its related to this issue however.

infines’s picture

So the patch in #70 is the answer.

patrickd’s picture

Had the same issue, tried:

applied patch of #70
Progressbar is displayed now but upload hangs on "Starting upload..."

applied patch of #97
Now only progressbar is shown but still hanging and I get an AJAX error:

An AJAX HTTP error occurred. HTTP Result Code: 200 Debugging information follows. Path: /file/progress/1423590844 StatusText: OK ResponseText: {"message":"Starting upload...","percentage":-1}[{"command":"settings","settings":{"basePath":"\/","pathPrefix":"","ajaxPageState":{"theme":"bartik","theme_token":"am1k-9rUAJLgRttbMdUdW6LwhrChTcf512AJiYlnBMQ","css":[]},"admin_menu":{"destination":"destination=file\/progress\/1423590844","hash":"760113b4ef62dded490eaf6e11662161","basePath":"\/admin_menu","replacements":{".admin-menu-users a":"0 \/ 1"},"margin_top":1}},"merge":true}]

Used a clean installation with bartik theme.
Hosting on acquia devcloud with PECL extension enabled.

droplet’s picture

@patrickd,
patch of #97 only affect APC, so you should get same result of #70 & #97.
Suggest reinstall drupal again, patch it with #70 and clear caches. Post your PECL version if you still get failed.

see #113, maybe same problem ?

patrickd’s picture

I just tested #97 to tell you how/wether its working on my server
I applied #70 on a absolutelly clean install and cleared cache without success (same problems to #120)
upload progress version: 1.0.1

xavierraffin’s picture

patch #70 works fine for me (D 7.8).

I have several D6 Drupal web sites which works on same server (with PECL progress bar).
So maybe, issue #120 is due to bad PECL conf ?

JordanMagnuson’s picture

#97 works fine for me (D 7.8 pecl upload_progress 1.0.3.1).

droplet’s picture

Assigned: droplet » Unassigned
infines’s picture

This needs to be commited.

catch’s picture

Before it gets committed it needs someone to reroll per the feedback in #101, until someone does that posting on this issue is not very helpful, perhaps you'd like to try the re-roll?

infines’s picture

I'm afraid I don't have the proper tools to do that at the current point in time, if someone, anyone could reroll this patch that would be great.

droplet’s picture

Status: Needs work » Needs review
FileSize
2.85 KB

reroll patch:
- new /CORE dir
- changed comment message (#103)

(Please reroll a new patch instead just leave a comment here if you think the comment or other part of this patch is not good enough, it helps this issue move forward)

brainbite’s picture

Version: 8.x-dev » 7.x-dev

I'm new to Drupal. And so far my experienced was pretty amazing.

But I see this issue as very critical!
Large file uploads are not possible without a progress bar!
No one waits 10 minutes before his screen looking on the trobber icon.

Too bad I'm buidling a site for a video contest where large files are submitted.
Not sure what I will tell my costumers tomorrow.

Any chance the progress bar will EVER be working in Drupal 7 ?

infines’s picture

Version: 7.x-dev » 8.x-dev

Yes, that patch should work now with Drupal 7. However it needs to be applied to Drupal 8 before it can be committed to Drupal 7.

infines’s picture

That patch works for me perfectly, using drupal 7.9

The patch should be submitted and anyone with issues with the feature should open them up separately as the patch seems to be working for the majority of testers.

The patch doesn't work however when the upload buttons aren't clicked but the save button on a node is instead clicked by the end user. Clicking save should execute the progress bar ajax actions also.

infines’s picture

Status: Needs review » Reviewed & tested by the community
catch’s picture

Issue tags: +Novice

@gridbitlabs: please open a new bug report for the save button issue and link to it from here.

The comment looks better, but two comments at the end of the patch file are missing periods on the end and don't wrap at 80 characters. I'm adding the Novice tag for a re-roll - I could fix on commit, but then webchick would have to do the same thing for Drupal 7 as well, so might as well just fix it now.

+        // Uploadprogress extension requires this field to be at the top of the form
catch’s picture

Status: Reviewed & tested by the community » Needs work
droplet’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
2.88 KB
catch’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Perfect crosspost, thanks!

Committed/pushed to 8.x, moving to 7.x. This will need a minor re-roll for 7.x as well.

droplet’s picture

Status: Patch (to be ported) » Needs review
FileSize
2.82 KB

Status: Needs review » Needs work

The last submitted patch, 935208-uploadprogress-bar-fix-129-reroll.patch, failed testing.

catch’s picture

@droplet - you'd need to upload the D7 patch without the D7 suffix for the test bot to not skip it.

droplet’s picture

Status: Needs work » Needs review
FileSize
2.82 KB
2.82 KB
Tor Arne Thune’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
1.73 KB

I can confirm that 935208-uploadprogress-bar-fix-129-reroll_0.patch is just a re-roll for D7 of 935208-uploadprogress-bar-fix-70-reroll.patch. Interdiff attached.
Still applies cleanly to D7.x.

Dane Powell’s picture

I can confirm that #141 works on 7.9, using APC 3.1.9 and PECL uploadprogress.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

The only unfortunate thing about this from a D7 backport standpoint is that it changes a hook_menu() definition, which results in a nasty error when you hit the upload button, until you clear the cache.

However, since #1049284: Running update.php to flush caches no longer works unless there are updates pending was just committed, hopefully this won't cause too many issues in the field.

Committed and pushed to 7.x. Thanks!

daigorocub’s picture

I tested all patches and none works. I see the progress bar saying "Starting upload", there's some ajax running, but the result is always the same:
{"message":"Starting upload...","percentage":-1}

I'm using drupal 7.9, PECL 1.0.1 and tested also with 1.0.3.1.

Please advise!

infines’s picture

My advice is to wait for the next release of drupal which will have the fixes built in. Then clear your cache twice. Try that with these patches. If all fails then, open up a new issue against core about this. Next drupal point release is the 11/30/11

infines’s picture

Also I advise that extension=uploadprogress.so is enabled in php.ini

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

droplet’s picture

Great.

Drupal 7.10 included this patch. do a fresh installation if you're failed :)

- try D7.10
- try new version of PECL
- try APC

and report as a new thread if you still hit the problems :)

j0rd’s picture

nvm

j0rd’s picture

Issue summary: View changes

Updated issue summary.

mendu.sriram’s picture

Upload progress not working in content type loaded through Ajax...
I have trace response
{"message":"Starting upload...","percentage":-1}

can anybody help me please ...