Follow-up of #1664940: [Policy, patch] Decide on JSHint configuration and part of #1415788: Javascript winter clean-up

Run jshint on the files with the configuration from the parent issue or use jshint.com with the following options:

/*jshint forin:true, noarg:true, eqeqeq:true, undef:true, curly:true, browser:true, expr:true, latedef:true, newcap:true, trailing:true */
/*global Drupal, jQuery */

Fix any warnings or errors the tool finds.
Check manually that the fixes did not break any functionalities
Create patch and upload for the testbot.

Files: overlay/overlay-child.js, overlay/overlay-parent.js

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nod_’s picture

core/modules/overlay/overlay-child.js: line 21, col 18, 'settings' is already defined.
core/modules/overlay/overlay-child.js: line 191, col 63, eval is evil.
core/modules/overlay/overlay-child.js: line 193, col 22, Missing radix parameter.
core/modules/overlay/overlay-parent.js: line 391, col 12, Missing 'new' prefix when invoking a constructor.
core/modules/overlay/overlay-parent.js: line 459, col 49, Missing radix parameter.
core/modules/overlay/overlay-parent.js: line 479, col 27, Missing radix parameter.
core/modules/overlay/overlay-parent.js: line 580, col 9, Bad line breaking before '?'.
core/modules/overlay/overlay-parent.js: line 796, col 12, Don't make functions within a loop.
droplet’s picture

Status: Active » Needs work
FileSize
3.46 KB
core/modules/overlay/overlay-child.js: line 191, col 63, eval is evil.
core/modules/overlay/overlay-parent.js: line 617, col 9, Empty block.
core/modules/overlay/overlay-parent.js: line 794, col 12, Don't make functions within a loop.

eval fixing in other patch.
function within a loop, I would like to do more test before patch :)

nod_’s picture

Got rid of the empty block warning. We can ignore the evil warning for now since it'll be taken care of by #1574738: Rewrite tableheader.js.

Not directly related to JSHint but there was a new missing, which sucks.

droplet’s picture

Status: Needs work » Needs review
FileSize
4.26 KB
sxnc’s picture

Ran through JSHint for the latest patch and fixed the core/modules/overlay/overlay-parent.js: line 794, col 12, Don't make functions within a loop.

The issue from core/modules/overlay/overlay-child.js: line 191, col 63, eval is evil. is ignored as stated in #3

frega’s picture

Status: Needs review » Reviewed & tested by the community

After applying patch from #1740958: overlay is broken (http://drupal.org/files/overlay-fix-1740958-1.patch) and applying patch from comment 13 jshint "passes" ("eval is evil" is ignored as stated in #3) and overlay still works as expected.

jshint ./core/modules/overlay/*.js
./core/modules/overlay/overlay-child.js: line 191, col 63, eval is evil.
1 error
nod_’s picture

Status: Reviewed & tested by the community » Needs work

Need reroll, eval was taken out of core \o/

frega’s picture

Status: Needs work » Needs review
FileSize
5.2 KB

Re-rolled patch from #5. Overlay works. Code looks good. jshint doesn't complain.

droplet’s picture

+++ b/core/modules/overlay/overlay-parent.jsundefined
@@ -776,6 +772,16 @@ Drupal.overlay.fragmentizeLink = function (link, parentLocation) {
+/* Refreshing a specific region */

catch a comment style issue. I think drupal only accepts

// single line

or

/**
 * multiple line ..etc
 */
nod_’s picture

Status: Needs review » Needs work

you're right droplet

(edit) other than that it should be good to go.

frega’s picture

I happily oblige :) Re-rolled with a multi-line docblock.

frega’s picture

Status: Needs work » Needs review

Sorry, forgot state change.

nod_’s picture

Status: Needs review » Reviewed & tested by the community

awesome, thanks :)

catch’s picture

Status: Reviewed & tested by the community » Fixed

Looks good. Committed/pushed to 8.x.

Status: Fixed » Closed (fixed)

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

nod_’s picture

Status: Closed (fixed) » Needs work
core/modules/overlay/overlay-parent.js: line 886, col 72, Missing radix parameter.
droplet’s picture

Status: Needs work » Closed (fixed)

Cannot find any error, checkout new repo, thanks.

nod_’s picture

Status: Closed (fixed) » Needs work

New JSHint config #1995996: Update JSHint configuration.

core/modules/overlay/overlay-child.js: line 145, col 11, 'id' is defined but never used.

core/modules/overlay/overlay-parent.js: line 473, col 11, '_tmp' is defined but never used.
core/modules/overlay/overlay-parent.js: line 770, col 51, 'regionName' is defined but never used.
core/modules/overlay/overlay-parent.js: line 770, col 67, 'regionSelector' is defined but never used.
droplet’s picture

Status: Needs work » Needs review
FileSize
1.49 KB
nod_’s picture

Status: Needs review » Reviewed & tested by the community

tested, works fine.

Had doubts about setInterval thing. Turns out it is fine since it's canceled because the page inside the iframe reloads.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 361964b and pushed to 8.x. Thanks!

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