#2533498: Update jQuery to version 3 might not happen for Drupal 8.3, but we should at least get onto the latest 2.x release. We're on 2.2.3 currently, but 2.2.4 has been out since May 2016.

CommentFileSizeAuthor
#9 2852636-6-9-interdiff.txt470 byteskporras07
#9 2852636-update-jquery-9.patch424.93 KBkporras07
#6 2852636-3-6-interdiff.txt376 bytesgnuget
#6 2852636-update-jquery-6.patch424.76 KBgnuget
#3 2852636-update-jquery-3.patch424.39 KBgnuget
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

effulgentsia created an issue. See original summary.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Needs work » Active

Thanks @effulgentsia. This is indeed patch-release-eligible since it's a patch-level update.

gnuget’s picture

gnuget
Primary language Spanish
CreditAttribution: gnuget as a volunteer and at Agaric commented
Status: Active » Needs review
FileSize
2852636-update-jquery-3.patch424.39 KB

Hi!

Patch attached.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Looks good to me, changelog is small. Clicking around show things still work.

droplet’s picture

droplet CreditAttribution: droplet commented
Status: Reviewed & tested by the community » Needs work

Needs to update core.libraries.yml

gnuget’s picture

gnuget
Primary language Spanish
CreditAttribution: gnuget as a volunteer and at Agaric commented
Status: Needs work » Needs review
FileSize
2852636-update-jquery-6.patch424.76 KB
2852636-3-6-interdiff.txt376 bytes

duh!

Thanks for your reviews.

New patch attached.

nod_’s picture

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

Path to the licence to update too.

gnuget’s picture

gnuget
Primary language Spanish
CreditAttribution: gnuget as a volunteer and at Agaric commented
Issue tags: +Novice
kporras07’s picture

kporras07 CreditAttribution: kporras07 at Manatí commented
Status: Needs work » Needs review
FileSize
2852636-update-jquery-9.patch424.93 KB
2852636-6-9-interdiff.txt470 bytes

Attached new patch.

Btw, it looks like the jquery license file previously listed doesn't exist since so much time ago.

swarad07’s picture

swarad07
Primary language English
Location Navi Mumbai, India
CreditAttribution: swarad07 at Axelerant commented

While applying patch, there are warnings about tab indentation. I am assuming we cannot do much about it as it is a vendor js file.

<stdin>:34: tab in indent.
	isSimulated: false,
<stdin>:42: tab in indent.
		if ( e && !this.isSimulated ) {
<stdin>:51: tab in indent.
		if ( e && !this.isSimulated ) {
<stdin>:60: tab in indent.
		if ( e && !this.isSimulated ) {
<stdin>:88: tab in indent.
  1. +++ b/core/assets/vendor/jquery/jquery.js
@@ -65,7 +65,7 @@ var support = {};
-	version = "2.2.3",
+	version = "2.2.4",

    Tab added 

  2. +++ b/core/assets/vendor/jquery/jquery.js
@@ -5006,13 +5006,14 @@ jQuery.Event.prototype = {
+	isSimulated: false,

    Tab added 

  3. +++ b/core/assets/vendor/jquery/jquery.js
@@ -5006,13 +5006,14 @@ jQuery.Event.prototype = {
-		if ( e ) {
+		if ( e && !this.isSimulated ) {

    Tab added 

  4. +++ b/core/assets/vendor/jquery/jquery.js
@@ -5021,7 +5022,7 @@ jQuery.Event.prototype = {
-		if ( e ) {
+		if ( e && !this.isSimulated ) {

    Tab added 

  5. +++ b/core/assets/vendor/jquery/jquery.js
@@ -5030,7 +5031,7 @@ jQuery.Event.prototype = {
-		if ( e ) {
+		if ( e && !this.isSimulated ) {

    Tab added 

  6. +++ b/core/assets/vendor/jquery/jquery.js
@@ -7864,6 +7852,7 @@ jQuery.extend( jQuery.event, {
+	// Used only for `focus(in | out)` events

    Tab added

droplet’s picture

droplet CreditAttribution: droplet commented
Status: Needs review » Reviewed & tested by the community

@swarad07,

we need not fix the vendor formatting.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: 2852636-update-jquery-9.patch, failed testing.

gnuget’s picture

gnuget
Primary language Spanish
CreditAttribution: gnuget as a volunteer and at Agaric commented
Status: Needs work » Reviewed & tested by the community

  • xjm committed 90bdd03 on 8.4.x 
    Issue #2852636 by gnuget, kporras07: Update jQuery to version 2.2.4
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Fixed
Issue tags: +8.3.0 release notes

Thanks everyone for the update and JS review!

After discussing the patch-level update with @wimleers, I think it's best to get this in now and use the RC phase to flush out any bugs that might occur.

Note that the vendor whitespace is correctly skipped by our coding standards checking, so we are good there:

/Users/xjm/git/maintainer/core/assets/vendor/jquery/jquery.js
  0:0  warning  File ignored because of a matching ignore pattern. Use "--no-ignore" to override

✖ 1 problem (0 errors, 1 warning)

ESLINT: core/assets/vendor/jquery/jquery.js passed

/Users/xjm/git/maintainer/core/assets/vendor/jquery/jquery.min.js
  0:0  warning  File ignored because of a matching ignore pattern. Use "--no-ignore" to override

✖ 1 problem (0 errors, 1 warning)

ESLINT: core/assets/vendor/jquery/jquery.min.js passed
 rewrite core/assets/vendor/jquery/jquery.min.map (92%)

Committed to 8.4.x and cherry-picked to 8.3.x. Thanks!

  • xjm committed 7516c7a on 8.3.x 
    Issue #2852636 by gnuget, kporras07: Update jQuery to version 2.2.4

(...

Status: Fixed » Closed (fixed)

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