Posted by nod_ on July 14, 2012 at 2:16pm
7 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | javascript |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | JavaScript clean-up |
Issue Summary
Follow-up of #1664940: [Policy, patch] Decide on JSHint configuration and part of #1415788: [Meta] javascript spring 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: shortcut/shortcut.admin.js
Comments
#1
core/modules/shortcut/shortcut.admin.js: line 55, col 75, 'self' is not defined.core/modules/shortcut/shortcut.admin.js: line 55, col 95, 'self' is not defined.
#2
#3
I'd prefer we don't add a new variable since replacing
selfwiththiswould work just as well.#4
#5
works for me
#6
Committed to 8.x. Thanks.
#7
I guess in general y'all aren't backporting these JSHint patches to Drupal 7 (which makes sense)... but this one seems like a straight-up bugfix, no?
#8
I'd rather to hardcode it than using global (default) setting.
var changedRowObject = new tableDrag.row(changedRow, 'mouse', false, 0, true);#9
That's not a bug actually just sloppy code.
selfwill resolve towindow.selfand both will be undefined. Meaning they will coerce to false in all the conditions tabledrag has on them. Basically it "just works"™ and we should hardcode it like droplet proposed, since this will not have any hierarchy ever.(In D8 all this code is gone now thanks to the remove shortcut limit patch)
#10
#11
#12
it's a bug fix and not the backports. D8 removed that part of code from other patches.
#13
Since it seems David already approves of this for 7.x, committed and pushed to 7.x. Thanks!
#14
Automatically closed -- issue fixed for 2 weeks with no activity.