WordPress.org

Make WordPress Core

Opened 5 months ago

Closed 5 months ago

Last modified 5 months ago

#26041 closed defect (bug) (fixed)

jshint shouldn't throw errors: wp-includes/js/plupload/handlers.js

Reported by: mdbitz Owned by: nacin
Milestone: 3.8 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords:
Focuses: Cc:

Description (last modified by SergeyBiryukov)

jshint cleanup of wp-includes/js/plupload/handlers.js

W109 : Strings must use singlequote.
W041 : Use '===' to compare with '0'
W069 : ['resize'] is better written in dot notation
W033 : Missing semicolon
W014 : Bad line breaking before '+'
W117 : ['post_id', 'pluploadL10n', 'resize_width', 'resize_height', 'shortform', 'ajaxurl', 'type', 'deleteUserSetting', 'setUserSetting', 'plupload'] is not defined

Attachments (1)

26041.patch (3.5 KB) - added by mdbitz 5 months ago.
initial jshint cleanup

Download all attachments as: .zip

Change History (6)

mdbitz5 months ago

initial jshint cleanup

comment:1 mdbitz5 months ago

Hi i've been reviewing the code however I have some questions on the remaining items identified by jshint.

1) Should the following bind function be removed? or should it be wrapped in a jshint comment to ignore the error?

uploader.bind('BeforeUpload', function(up, file) {
    // something
});

2) Should we remove parameters or add jshint ignore comments on items that are to-do items?

function deleteError(X, textStatus, errorThrown) {
	// TODO
}

3) JShint is saying fileDialogStart and dndHelper are defined but not used. What is the correct metod to move ahead with clearing of these errors.

comment:2 nacin5 months ago

This is an old file. Its structure (if you can call it that) came from swfupload/handlers.js. We copied that and then changed it up for Plupload in 3.3. In 3.4, a new API was written for the customizer, wp-includes/js/plupload/wp-plupload.js. In 3.5, that API was updated and used for the new media library. So this is only used for media-new.php. It would be nice to eventually delete these files, for sure. With that in mind, it would be best if this file is left as pristine as possible for code spelunking purposes. So:

  1. Let's comment out the BeforeUpload.
  1. Let's remove the parameters. If we ignore the entire function, we'll then need to remove the reference to deleteError as well.
  1. Normally we would mark these as "exported". It looks like dndHelper is *not* from SWFUpload, and instead came from the project that added Plupload to core. It was accidental, essentially, and we can yank it. fileDialogStart was used only through 3.2 when we had SWFUpload; it was copied over to here in 3.3. Anyone trying to use it will be using the SWFUpload handlers, which includes a function of the same name. It can also be removed.

comment:3 nacin5 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 26205:

Fix JSHint errors in plupload handlers.js.

props mdbitz.
fixes #26041.

comment:4 nacin5 months ago

  • Milestone changed from Awaiting Review to 3.8

comment:5 SergeyBiryukov5 months ago

  • Description modified (diff)
Note: See TracTickets for help on using tickets.