I can't find any piece of code using this.progress.update_callback and this.progress.error_callback on this line: ajax.js

My google-fu and ctrl-F fu is not strong enough.

The way this thing works is that a PHP settings gives the name of a JS function that is used as callback. It's wrong, that's not how you use javascript and If this needs to be supported and there is actual code using it, I'd be happy to do it properly (this could use some jquery ui to make it work).

Here is a patch that gets rid of it, it uses eval() and gets in the way of #1417378: Remove eval() from tableHeader JavaScript and #1415788: Javascript winter clean-up.

CommentFileSizeAuthor
#7 core-js-remove-ajax-dead-code-1420022-7.patch698 bytesnod_
core-js-remove-ajax-dead-code.patch1.45 KBnod_
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Tor Arne Thune’s picture

Tor Arne Thune CreditAttribution: Tor Arne Thune commented

There was a commit not long ago that fixed the uploadprogress bar: #935208: PECL uploadprogress bar doesn't appear.. Maybe it made the code you mention obsolete.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Issue tags: +JavaScript clean-up

I'm not sure, but it's definitely related.

I can still use the progressbar for my file upload. It's the closest I've been to related code, that's already a win, thanks.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Active » Needs review

I need a review because I can't find anything more about this. For me this has to go.

Niklas Fiekas’s picture

Niklas Fiekas CreditAttribution: Niklas Fiekas commented

The default error message for machine_name fields would be: The machine-readable name must contain only lowercase letters, numbers, and underscores.. (Mhh ... wrong issue.)

Niklas Fiekas’s picture

Niklas Fiekas CreditAttribution: Niklas Fiekas commented
Title: Remove dead code: progress.upload_callback, progress.error_callback » Remove dead code: progress.updateCallback, progress.errorCallback
Status: Needs review » Needs work

Fixing method names in issue title.

With this patch admin/reports/updates/check stops working. If you search for updateCallback and errorCallback in core/misc/batch.js you'll find that it looks like it's beeing used there. But I am not sure enough to mark this won't fix.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Title: Remove dead code: progress.updateCallback, progress.errorCallback » Remove dead code: progress.upload_callback, progress.error_callback

Oh yeah I ended up breaking that indeed.

What I want to remove is the eval part, I don't want to take out the methods you're talking about because it could be used, that's a valid thing to have.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Needs work » Needs review
FileSize
core-js-remove-ajax-dead-code-1420022-7.patch698 bytes
Niklas Fiekas’s picture

Niklas Fiekas CreditAttribution: Niklas Fiekas commented
Status: Needs review » Reviewed & tested by the community

Looks good. Throbbers and progress bars in core are still working.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Issue tags: +API change

Looks good to me.

We discussed in IRC, even asked @merlinofchaos and others whether anyone uses this dynamic callback, and apparently no one even knew that this even existed. The code dates back to ahah.js.

That said, it's still an API change, so we should at least write a small change notice.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks all!

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Fixed » Reviewed & tested by the community
Issue tags: +Needs change record

Looks like this commit didn't actually make it in. Also still needs that change notification that sun suggests.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Title: Remove dead code: progress.upload_callback, progress.error_callback » Change notification: Remove dead code: progress.upload_callback, progress.error_callback
Priority: Normal » Critical
Status: Reviewed & tested by the community » Active

Alright, the commit is there now, so change notification time.

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
CreditAttribution: gdd commented
Status: Active » Needs review

http://drupal.org/node/1451282

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Fixed

Thanks @heyrocker!

Tor Arne Thune’s picture

Tor Arne Thune CreditAttribution: Tor Arne Thune commented
Title: Change notification: Remove dead code: progress.upload_callback, progress.error_callback » Remove dead code: progress.upload_callback, progress.error_callback
Priority: Critical » Normal

Status: Fixed » Closed (fixed)

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

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Issue tags: -Needs change record
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Issue summary: View changes

add jquery ui comment