Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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
Comment | File | Size | Author |
---|---|---|---|
#19 | overlay.jshint.patch | 1.49 KB | droplet |
#11 | 1684862-core-js-jshint-overlay-10.patch | 5.24 KB | frega |
#11 | 1684862-core-js-jshint-overlay-8-10.interdiff.txt | 617 bytes | frega |
#8 | 1684862-core-js-jshint-overlay-8.patch | 5.2 KB | frega |
#5 | core-js-jshint-overlay-1684862-5.patch | 5.63 KB | sxnc |
Comments
Comment #1
nod_Comment #2
droplet CreditAttribution: droplet commentedeval fixing in other patch.
function within a loop, I would like to do more test before patch :)
Comment #3
nod_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.Comment #4
droplet CreditAttribution: droplet commentedComment #5
sxnc CreditAttribution: sxnc commentedRan 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 #3Comment #6
frega CreditAttribution: frega commentedAfter 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.
Comment #7
nod_Need reroll, eval was taken out of core \o/
Comment #8
frega CreditAttribution: frega commentedRe-rolled patch from #5. Overlay works. Code looks good. jshint doesn't complain.
Comment #9
droplet CreditAttribution: droplet commentedcatch a comment style issue. I think drupal only accepts
or
Comment #10
nod_you're right droplet
(edit) other than that it should be good to go.
Comment #11
frega CreditAttribution: frega commentedI happily oblige :) Re-rolled with a multi-line docblock.
Comment #12
frega CreditAttribution: frega commentedSorry, forgot state change.
Comment #13
nod_awesome, thanks :)
Comment #14
catchLooks good. Committed/pushed to 8.x.
Comment #16
nod_Comment #17
droplet CreditAttribution: droplet commentedCannot find any error, checkout new repo, thanks.
Comment #18
nod_New JSHint config #1995996: Update JSHint configuration.
Comment #19
droplet CreditAttribution: droplet commentedComment #20
nod_tested, works fine.
Had doubts about setInterval thing. Turns out it is fine since it's canceled because the page inside the iframe reloads.
Comment #21
alexpottCommitted 361964b and pushed to 8.x. Thanks!