AJAX error when using progress bar on file field widget

Found this error yesterday, didn't have time to fix it as it was not a blocker for my project stage. Now that I was just polishing figured I should fix it, but might as well use this issue to avoid duplicates.

The problem was that the route wasn't configured to pass the key to the controller.

This does cause an error on file upload - doesn't actually interfere with the upload but still looks bad, so I think it should get in before the 19th.

Can someone please post the server-side error in the issue summary?

Here's a copy of a log message:
Location http://example.ca/file/progress?_format=json
Referrer http://example.ca/file/add
Message RuntimeException: Controller "Drupal\file\Controller\FileWidgetAjaxController::progress()" requires that you provide a value for the "$key" argument (because there is no default value or because there is a non optional argument after this one). in Drupal\Core\Controller\ControllerResolver->doGetArguments() (line 171 of /var/www/example.ca/public_html/core/lib/Drupal/Core/Controller/ControllerResolver.php).
Severity Error

Thanks for the bug report!

Needs tests probably?

Needs tests how? Do we have AJAX tests already? Isn't this something after step2 of TNG'ing tests? dawehner is working on getting phantomjs to drupalci.

@chx, we have been able to test the server side for AJAX requests and responses for awhile, even though we can't test the JavaScript. See WebTestBase::drupalPostAjaxForm(). Since this is a server-side error, it should indeed be testable I think.

So you can test path: '/file/progress/{key}' this hunk but not this $element['upload_button']['#ajax']['progress']['url'] = Url::fromRoute('file.ajax_progress', ['key' => $upload_progress_key]); . I guess that's worth a test, sure.

Test for '/file/progress/{key}'.

Fixing typo.

+++ b/core/modules/file/src/Tests/FileFieldWidgetTest.php
@@ -40,7 +40,7 @@ protected function setUp() {
+  public static $modules = array('comment', 'block', 'locale');

Why is locale needed in the test modules?

Why is locale needed in the test modules?

Pardon my ignorance. :D

Took me a while to get this configured, but yes, this patch fixes the error. I didn't see any real 'progress' though while uploading a 700 MB file, I thought I'd see some message with percentage or so? In the end, the file as uploaded though, so maybe for a follow up ?
Moving to RTBC for feedback from committers.

The 'Internal Server Error' is gone, however the progress meter always get a -1 in response using PECL upload progress on PHP 5.6.

I've tested upload progress on the same server with Drupal 7, and it's example from https://github.com/php/pecl-php-uploadprogress/tree/master/examples and both worked.

I think this still needs work. ( I used path #15)

#19 implies there is more to do here to make the progress bar work properly here.

Hmm yeah, both uploadprogress_get_info() apcu_fetch() return nothing .. looking into it.

We also know there's no upload progress on the testbot, so we can't write a test for it (for now).

I'm not sure how this was solved in D7 to handle multiple file upload fields in a single form, but I think we need a hidden input using the same "key" as sent back to the server to track progress:
https://harperreed.com/writing/2007/04/16/pecl-uploadprogress-example/

I have never tried APCu, so don't know how it works.

The hidden field is there:

<div id="ajax-wrapper"><input class="file-progress" data-drupal-selector="edit-field-file-0-upload-identifier" type="hidden" name="field_file[0][UPLOAD_IDENTIFIER]" value="1347176024">
<input data-drupal-selector="edit-field-file-0-upload" type="file" id="edit-field-file-0-upload" name="files[field_file_0]" size="22" class="js-form-file form-file">

The code is more or less the same as in D7, so it's probably going to be something trivial.

- edit - works perfect on D7 btw

It's not entirely related, but just adding it because there's various problems with file uploads - #2346893: Duplicate AJAX wrapper around a file field

#2662778: Ajax progress is rendered via JavaScript theme function; does not include progress library is a spin off to fix the theming of the progress bar.

Ok, I have a hint why this is failing: compare

D7 post data

------WebKitFormBoundary4d4zgU831PUSggVA
Content-Disposition: form-data; name="UPLOAD_IDENTIFIER"

123487692

with

D8 post data

------WebKitFormBoundaryctCyV2fneyPxBbKl
Content-Disposition: form-data; name="field_file[0][UPLOAD_IDENTIFIER]"

426806279

The name is wrong in the D8 version. file.js tries to rename the name field temporarily (and it finds it), but it doesn't end up in the post data.

I've split the issue up in #2662932: Fix file upload progress bar, I think it's better to get this one in first because it's

a) an easy and straight bug fix
b) the other one really seems to be a js problem and if we want people to help out, then i'd be easier if the fatal is already in

So therefor, RTBC.

Committed/pushed to 8.1.x and cherry-picked to 8.0.x, thanks!

I am seeing this error with Drupal 7.56 and PHP71. Problem goes away with PHP70. Is there a issue post or fix for Drupal-7.x?

PHP version 7.2.0RC5 works OK!