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

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. 

core/modules/edit/js/views/toolbar-view.js: line 224, col 29, Expected '!==' and instead saw '!='.

core/modules/edit/js/backbone.drupalform.js: line 96, col 26, Bad escaping.
core/modules/edit/js/backbone.drupalform.js: line 96, col 35, Bad escaping.
CommentFileSizeAuthor
#12 core-jshint-edit-1955292-12.patch707 bytesnod_
#7 edit.patch1.32 KBdroplet
#3 errors.txt6.17 KBtmckeown
#3 jslint_fixes-1955292.patch9.3 KBtmckeown
#3 drupal_8_js_files.txt3.33 KBtmckeown
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Issue tags: +Novice, +JavaScript clean-up, +js-novice

tag

tmckeown’s picture

tmckeown CreditAttribution: tmckeown commented
Assigned: Unassigned » tmckeown

Taking this one as part of the Long Beach Drupal Ladders sprint tonight. To find all js files from the command line this command helped.

find . -type f -name \*.js
tmckeown’s picture

tmckeown CreditAttribution: tmckeown commented
Status: Active » Needs work
FileSize
drupal_8_js_files.txt3.33 KB
jslint_fixes-1955292.patch9.3 KB
errors.txt6.17 KB

Attached are the remaining error, the files checked, as well as a patch. I don't suggest we change modernizr and I wasn't not sure what the issue was with drupal.js

tmckeown’s picture

tmckeown CreditAttribution: tmckeown commented
Status: Needs work » Needs review
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Needs review » Needs work

Thanks for the patch!

To keep the sanity level as high as possible we're checking each module individually so the only files we want to change in this issue would be toolbar-view.js and backbone.drupalform.js, not collapsed.js and the rest, check out the table from this issue #1415788: Javascript winter clean-up All the files/modules are listed and issues are associated with each of them.

droplet’s picture

droplet CreditAttribution: droplet commented 
+++ b/core/misc/autocomplete.jsundefined
@@ -316,7 +319,9 @@ Drupal.ACDB.prototype.search = function (searchString) {
-        alert(Drupal.ajaxError(xmlhttp, db.uri));
+        if (window.console) {
+          window.console.log(Drupal.ajaxError(xmlhttp, db.uri));
+        }
       }

this is alert

droplet’s picture

droplet CreditAttribution: droplet commented
Status: Needs work » Needs review
FileSize
edit.patch1.32 KB

EDIT JS is broken, cannot do test :(

Status: Needs review » Needs work
Issue tags: -Novice, -JavaScript clean-up, -js-novice

The last submitted patch, edit.patch, failed testing.

droplet’s picture

droplet CreditAttribution: droplet commented
Status: Needs work » Needs review
Issue tags: +Novice, +JavaScript clean-up, +js-novice

#7: edit.patch queued for re-testing.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Needs review » Closed (works as designed)

I'm making sure the refactor of Edit JS doesn't have JSHint issues.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Closed (works as designed) » Needs work

New JSHint config #1995996: Update JSHint configuration.
core/modules/edit/js/views/AppView.js: line 269, col 26, 'targetState' is defined but never used.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Assigned: tmckeown » Wim Leers
Status: Needs work » Needs review
FileSize
core-jshint-edit-1955292-12.patch707 bytes

Think that might be a bug, giving it to Wim.

I think patch does what it's supposed to be doing.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Issue tags: -Novice, -js-novice +sprint, +in-place editing, +Spark

#12 looks correct. I'll review later.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Component: javascript » edit.module
Status: Needs review » Reviewed & tested by the community

Confirmed. This is a bug introduced by #1979784: Factor Create.js and VIE.js out of the Edit module. The consequence is that right now, if you click the "close" button if the field has been modified, and you click the "Save" button in the modal, it will still cause the contents to be discarded.

Thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 895e29f and pushed to 8.x. Thanks!

Nice... jshint finding and helping to fix bugs!

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Issue tags: -sprint

Indeed :)

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

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

other errors