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 functionality.
Create patch and upload for the testbot.
Files: ajax.js
Comments
Comment #1
nod_Comment #2
nod_got rid of the command, key, filtered ifs, settings, conditional expression.
Left with
alertwhich should be fixed by #1606362: Use throw when an ajax error occurs and extend JavaScript's Error object/#1232416: Drupal alerts "An AJAX HTTP request terminated abnormally" during normal site operation, confusing site visitors/editors.And a couple of make a function within a loop warnings.
Comment #3
pascalduez commentedHey,
tried to fix the latest warnings.
Used window.alert as a temporary solution, waiting for #1606362: Use throw when an ajax error occurs and extend JavaScript's Error object ?
Created a new function to be used into the behaviors loading loop.
Renamed response.arguments into response["arguments"] to circumvent the reserved word issue, without changing all the arrays keys down into ajax.inc. Okay ?
Comment #4
pascalduez commentedCorrected the details we talked about.
Formatting, no space between functions args.
Named function declaration.
Changed "arguments" (which is a JavaScript reserved word into "args" in ajax_command_invoke()
Changes "argument" (for consistency) into "arg" in ajax_command_css() and ajax_command_settings().
Comment #5
pascalduez commentedRemoved changes in ajax_command_css() and ajax_command_settings().
Comment #6
pascalduez commentedClosure arguments: removed spaces, inverted order. :)
Comment #7
nod_works good, thanks :)
Comment #9
nod_#6: core_js-jshint_ajax-1684788-6.patch queued for re-testing.
Comment #10
pascalduez commentedCorrected "arguments" change in testAjaxCommands().
Comment #11
nod_Comment #12
frega commentedjshint doesnt complain anymore. did a close reading review of the patch. basic functionality (checking ajax behaviors, via "tags" field in article node bundle). couldn't do a test of the beforeSerialize-bit because I couldn't find it being used in core it self. If you have a pointer, _nod, I'll do that.
Comment #13
seutje commentedYeah, I don't think core uses ajax forms anywhere. But the code didn't really change, just some variable names and syntax. The only real change is the hasOwnProperty which actually makes it more robust for when 3rd party scripts screw with base prototypes.
Comment #14
nod_It does, manage display options for fields, autocomplete, and some other places :)
Comment #15
seutje commentedoh ok, did a manual test to make sure, nothing broke, as expected.
Comment #16
jelle_sPatch passes Testswarm tests for autocomplete.
RTBC++
Comment #17
webchickCommitted and pushed to 8.x. Thanks!
In case you, like me, were wondering why we abbreviate "arguments" to "args" which we usually do not do, #4 points out that "arguments" is a reserved keyword.
Comment #19
nod_New JSHint config #1995996: Update JSHint configuration.
core/misc/ajax.js: line 87, col 78, 'message' is defined but never used.Comment #20
nod_it's using this.messages instead of messages now.
Comment #21
droplet commentedsimple one
Comment #22
alexpottCommitted dcfda14 and pushed to 8.x. Thanks!