Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 10 months ago

#2002 closed defect (bug) (fixed)

Gratuitous abuse of JavaScript's eval() function

Reported by: h3h Owned by:
Milestone: Priority: low
Severity: minor Version: 2.0
Component: Administration Keywords: bg|has-patch
Focuses: Cc:


See: http://xkr.us/js/eval

Some of the JS on the inline-uploading.php page was written without understanding of what eval() is or does. It is not necessary for string concatenation in the least and is instead very costly.

Attachments (2)

inline-uploading.diff (1.5 KB) - added by h3h 12 years ago.
inline-uploading.php.diff (3.6 KB) - added by ringmaster 12 years ago.
Uses arrays, removes eval()s

Download all attachments as: .zip

Change History (9)

12 years ago


#1 @skeltoac
12 years ago

  • Resolution set to invalid
  • Status changed from new to closed

Eval is a necessary evil in this circumstance because we are building a var name, not a string, and we need the value of that var name. This diff breaks the script.

#2 @skeltoac
12 years ago

If you really insist on eliminating evals, rewrite the PHP that constructs the string vars so that it constructs a multidimensional array, then use the argument (n) to locate the correct array element. IMO this is a waste of time as the script is already very light.

#3 @ringmaster
12 years ago

  • Keywords bg|has-patch added
  • Priority changed from high to low
  • Resolution invalid deleted
  • Severity changed from normal to minor
  • Status changed from closed to reopened


12 years ago

Uses arrays, removes eval()s

#4 @h3h
12 years ago

Sorry, my extreme bias against eval() getting in the way of checking the patch for validity.

Nice fix, ringmaster.

#5 @skeltoac
12 years ago

Indeed, nice fix, ringmaster!

#6 @ryan
12 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [3263]) Eliminate eval. Props ringmaster. fixes #2002

This ticket was mentioned in Slack in #core-editor by zakkath. View the logs.

10 months ago

Note: See TracTickets for help on using tickets.