Update jQuery to version 3

Updating issue metadata to reflect the situation :)

Since we update to latest lib available, and jQuery has a good track record of shipping before us we should get ready. They don't have a 1 year beta cycle :þ

Looking at the changelog I don't think that many things will break (haven't tried it yet though).

Probably need a jquery ui update at the same time.

Just got a "Uncaught TypeError: f.getClientRects is not a function" error on some quickedit dialog. It's still functional but the placement is wrong. Looked at jqueryui source, it's fixed in the git repo but no release with said fix. Need to wait for a jquery3 compatible jquery ui to update this.

In our code, I haven't seen anything breaking. Looks like we did a good job dodging bullets.

It'd be great to ship Drupal 8 with jQuery 3. Does someone have a contact at jQuery that could help us coordinate that?

I'll ping scott_gonzalez. In the meantime the github milestone shows it's not there yet. https://github.com/jquery/jquery/milestones

I'm thinking this is something we could call out in the release notes as "not yet, but targeting before release" maybe. It doesn't seem wise to replace a stable, working jQuery with an alpha jQuery in RC1. OTOH, I would hate to ship 8.0.0 with jQuery 2 if 3 is ready before we are. And even if it's merely close (at least beta, ideally RC), we might want to move 8.0.0 to it, or at least 8.1.0.

This isn't possible until there's a new tag for jQuery 3, since it's been months since the last one and there's no information anywhere about it.

Bumping to 8.1.x. If that new tag does arrive during release candidate, we can move it back.

jQuery3 has just been released http://blog.jquery.com/2016/06/09/jquery-3-0-final-released/

#2746011: Update jQuery to version 3

http://blog.jquery.com/2016/06/09/jquery-3-0-final-released/ lists BC breaks. For example .load() has been removed. devel and panels_ipe use that, for example. It's easy to fix, but it is a break.

OTOH, there are still relatively few D8 contrib modules and sites, so the pain to fix these relatively minor breaks will be fairly small, and will have the benefit that we're not left behind on jQuery 2.x for years to come.

Tough call. Does semver apply to APIs of the 3rd party libraries shipped with software?

Does semver apply to APIs of the 3rd party libraries shipped with software?

Technically, I think, yes, it does. If something is a break to the "public API" of a library, then I think it's also a break to the public API of the Drupal core package, if Drupal core's composer.json or bundled JS files are changed to use the later version of the library. https://www.drupal.org/core/d8-allowed-changes#minor lists patch- and minor- level library updates as allowed during Drupal minor updates, but does not mention how to handle major- level library updates, because those need to be evaluated case by case.

In the case of updating to jQuery 3, we might have an out: the .load() method mentioned in #17 and the other event aliases removed in jQuery 3, were all marked as deprecated in jQuery 1.8, and jQuery 2 was released as being API-compatible with 1.9. Which means these methods could be interpreted as not in the public API of jQuery 2 for semver purposes. Note that by the jQuery team bumping the major version number from 2 to 3, they're signaling that that isn't their interpretation, but as Drupal, I think we're free to interpret differently than them.

I don't know if there are other breaks in jQuery 3, which we'd need to also evaluate with respect to semver.

Something else to consider is that jQuery Migrate can be used to restore (some? all?) legacy APIs. So we could potentially either ship with this in 8.2 and then remove in 8.3, or leave it out of 8.2 and leave it to contrib modules that need it to add it as a dependency.

Tagging for release manager feedback.

With Symfony 3.0 we posted https://www.drupal.org/node/2403135

#2712647: Update Symfony components to ~3.2 shows that in practice Symfony 3.x has bc breaks which aren't just dropping deprecations from Symfony 2.8. We're trying to minimize those in the issue but even then, on impact vs. disruption it makes sense to update the version.

For me the same goes for jQuery 3.0 - I do think would be worth adding jQuery migrate in 8.2.x and removing it in either 8.3.x or 8.4.x, but unless there was serious breakage, staying on 2.* for the next 3-8 years would feel like shooting ourselves in the foot.

Removing "Needs release manager review" tag, because that was provided in #19.

I do think would be worth adding jQuery migrate in 8.2.x and removing it in either 8.3.x or 8.4.x

Given that comment, how about making this issue add jquery-migrate to core/assets/vendor and adding that as a JS asset within the jquery entry of core.libraries.yml? That way, it's always included when jquery.js is included.

We can then open a follow-up issue to either make it a separate library within core.libraries.yml or to remove it entirely from core, and make corresponding decisions for when to do either or both of those. But meanwhile, I think that would allow this issue to proceed right away with minimal (possibly 0, depending on how complete jquery-migrate is) BC breakage.

I think better to make jQuery library dependent on that new "migrate" library and change record to describe how contrib themes can disable that

Maybe for 8.2.x add new experimental module #2042165: Add a 'deprecated' module that includes deprecated functions only when enabled

and adding that as a JS asset within the jquery entry of core.libraries.yml

IMO it would be better to add jquery.migrate as a separate library, and let the jquery library depend on jquery.migrate. That provides the same guarantee. But then it's conceptually separate, which makes it a bit easier for sites with "better jQuery code" to omit the jquery.migrate library.

… which apparently is exactly what @andypost wrote :)

I think better to make jQuery library dependent on that new "migrate" library and change record to describe how contrib themes can disable that

+1. I agree that's better than #21. However, do we need a way to prevent someone from adding the jquery.migrate library directly without adding the jquery library? Not that there's a use case for that, just what code is needed to prevent it happening by mistake? I don't think we can have each library depend on the other, can we?

Maybe for 8.2.x add new experimental module #2042165: Add a 'deprecated' module that includes deprecated functions only when enabled

Yes. If/when we have that module, we can make that the module that creates the dependency. Until then, let's proceed with the dependency created in core.libraries.yml for now.

However, do we need a way to prevent someone from adding the jquery.migrate library directly without adding the jquery library?

I don't see why. More importantly: we can't. We don't have the concept of "private libraries" like we have "private services".

I don't see why.

See https://code.jquery.com/jquery-migrate-3.0.0.js. The JS does in fact depend on the jQuery object existing. Therefore, jquery.migrate should list jquery as a dependency, both for conceptual accuracy, and because including the former in a web page without including the latter results in a JS error.

I don't think we can have each library depend on the other, can we?

I checked, and currently we cannot. We get infinite recursion in LibraryDependencyResolver::doGetDependencies().

I therefore think we need to either add support for mutual (circular) dependencies, or make it all part of a single library, per #21.

which makes it a bit easier for sites with "better jQuery code" to omit the jquery.migrate library

Even if part of the jquery library, can't these sites hook_library_info_alter() out the jquery-migrate JS file?

Just noting that jquery.com promotes v3.1.0 as the recommended release on the home page. We should try and figure this out for 8.3.

I checked, and currently we cannot. We get infinite recursion in LibraryDependencyResolver::doGetDependencies().

I therefore think we need to either add support for mutual (circular) dependencies, or make it all part of a single library, per #21.

So what is the next step? Add support for circular dependency?

jQuery 1.12 got out this month, and does support jQuery 3, we're good to go (but need to update both at the same time).
http://blog.jqueryui.com/2016/09/jquery-ui-1-12-1/

Patch with jquery 3, no jquery ui update. Dialog are opening but not placed correctly on the page.

Problems with jquery ui latest version, upstream bug: https://bugs.jqueryui.com/ticket/15052#ticket

Split the jquery ui in it's own issue since it might be headache-y

We didn't get this done in time for Drupal 8.3 beta. Updating a library to a new major version is risky to do after beta and not allowed at all in a patch release. So this issue is now likely an 8.4 only issue.

From http://blog.jquery.com/2016/06/09/jquery-3-0-final-released/:

While the 1.12 and 2.2 branches will continue to receive critical support patches for a time, they will not get any new features or major revisions.

Hopefully "for a time" will be at least 7 more months or so, long enough for Drupal 8.4 to be released.

I opened #2852636: Update jQuery to version 2.2.4 to at least get Drupal 8.3 onto the latest 2.2 patch release in the meantime.

Should we postpone this one until #2809427: Update jQuery UI to 1.12 be fixed?

Yes let's postpone. If there's a good reason to combine the issue we could recombine, but this can't go in first as it is, so it's postponed for now.

Also bumping to critical since if we don't get this into 8.4.x we'll be shipping stable 8.x releases with an unsupported jQuery version for 6+ months.

I reopen it. Pre-testing & Identify the problem also part of patching and help it move forward.

- jQuery UI isn't the only problem I believed. The code involved show() / hide() / attr() / prop() / ..etc / may be broken also. Many of them we can fix it now without jQuery 3. We able to support jQuery 3 before we upgrade it in CORE! It's possible! For example, Bootstrap theme replaced jQuery UI. They don't need jQuery UI stuff..

- Find out the compatible states of 3rd parties in CORE

The whole world is changing. In the old day, when jQuery release a new version, all 3rd parties push an update. Now what I feel, many libs just deferred the fixes. Or a totally rewritten into Plain JavaScript.

We need to make our CORE compatible with a range of jQuery (2.x ~ 3.x+) when we can. Let's fine-grained it now. (Sometimes, bugs exposed we have bad code, not just jQuery specified)

EDITED:

I don't know how. But I think it's good to announce jQuery 3 is coming into D8.4 or so. Drupal Contribs can support it before jQuery 3 get into CORE also.

Here's a patch upgrading to the latest recommended version on jquery.com, 3.2.1

Looking at the test fails there are lots of comments relating to getClientRects()

and referencing the migration guide

https://jquery.com/upgrade-guide/3.0/

Selector

link Breaking change: Behavior of :hidden and :visible

An element is considered now visible if it has a layout box returned from the DOM getClientRects() method, even if that box has a height and/or width of zero. This means that elements such as
or an empty element that don't have height are considered to be visible.

Looks like the failures are related to jquery ui, see https://github.com/jquery/jquery/issues/3157

I have ran a couple of the failing tests, Drupal\FunctionalJavascriptTests\Ajax\AjaxTest, and
Drupal\FunctionalJavascriptTests\Dialog\DialogPositionTest on my local machine, with both this patch and the one for jquery ui applied (#2809427: Update jQuery UI to 1.12), and both tests pass, so perhaps there is hope :)

Uploading here both of them combined just to see what the bot finds out.

Tried a few of these just now, AjaxCssTest, AjaxTest and CKEditorIntegrationTest pass locally, but fail on here so I must be missing something...

...missing indeed:

exception: [Warning] Line 1134 of core/modules/locale/locale.module:
file_get_contents(core/assets/vendor/jquery.ui/ui/data-min.js): failed to open stream: No such file or directory
...
exception: [Warning] Line 1134 of core/modules/locale/locale.module:
file_get_contents(core/assets/vendor/jquery.ui/ui/widgets/sortable-min.js): failed to open stream: No such file or directory

OK that patch came out wrong, thanks @tacituseu for helping clear it out. Let's try it again...

The non-combined patch that we'll eventually want to review looks great (the one in #42).

I don't see any fixes for the BC breaks though (see #17).

Working on manual testing.

I applied the combined patch and manually tested:

• /node/add/article (editor+ckeditor+dialogs+file uploads)
• /node/17/edit (same)
• /node/17 (contextual + quick edit + ckeditor)
• /admin/config/content/formats/manage/basic_html (filter + editor + ckeditor)
• /admin/config/content/formats/add (filter + editor + ckeditor)
• /admin/structure/taxonomy/manage/tags/overview (table drag)

Found no problems.

I also went over everything in http://jquery.com/upgrade-guide/3.0/. Good news: all of the things that stand out, the things that used to be pretty common … we already fixed those in Drupal core a long time ago. Here are some key examples:

1. .toggleClass() with either no arguments or a boolean is no longer supported — but we never used that anyway, precisely because it was confusing in the first place
2. .load() + .unload() + .error() + .bind() + .delegate() are gone, .on() is the successor — we have only been using the latter since long before D8 shipped
3. .andSelf() is gone, .addBack() is the successor — we have zero uses of the former, half a dozen of the latter

I'd RTBC this (well, the non-combined patch in #42), but this is blocked on #2809427: Update jQuery UI to 1.12 landing first, so marking postponed.

That's awesome news, thanks a ton @Wim Leers !

Some notes,

1. if (typeof response === 'string') {
     response = $.parseJSON(response);
   }
   

   From ajax.es6.js. jQuery's parseJSON method is being deprecated.

2. From ajax.es6.js, autocomplete.es6.js, ckeditor.admin.es6.js, plugin.es6.js, KeyboardView.es6.js, FieldDecorationView.es6.js, ToolbarVisualView.es6.js, there is a breaking change around how jQuery stores data attributes, camelCase vs. kebab-case.
3. .width(), .height(), .css("width"), and .css("height") may not always return a numeric value. These are used in a few places,

   dialog.position.es6.js, contextual.es6.js, off-canvas.es6.js, EntityToolbarView.es6.js, dialog.views.es6.js,
    tableheader.es6.js, color.es6.js

4. .outerWidth(), and .outerHeight() now include the scrollbar width and height. While probably not a massive issue, this can introduce subtle breaks when comparing these values with fixed items.
5. :hidden, and :visible have subtle changes that might effect tabledrag.es6.js. I haven't fully tested this, and it might be fine, or not immediately obvious.

1. Good catch, but deprecated !== breaking change :)
2. I checked them, they're not affected.
3. I checked those calls, they're not affected.
4. I missed that one.
5. I tried to test this manually, but couldn't even figure out what the :visible and :hidden were used. There be dragons in tabledrag.js.

#56.1 - We should probably still stop using that, but you are correct, not critical.
#56.4 - I don't even know if this is really an issue, some manual testing is probably required.
#56.5 - Yep, without some manual step-through debugging we won't know what those selectors return and if the changes to :hidden, and :visible break things.

#56.5
i did some work toward #2102677: Port tabledrag_example module to Drupal 8.. it may have to do with roots and leaves in tabledrag's

- Toolbar is broken (Toggle open/close/verticle/horizontal)
- Tour is broken (Tested: Language)

@droplet++
@droplet++
@droplet++
@droplet++

Thanks! That obviously means this can't be RTBC yet.

#2894283: Improve Toolbar Height calculation to accommodate jQuery 3.0 changes.

Well, I wonder if it's a bug in jQuery 3. This change makes jQuery less productive.

That means every selector involved height(), width(), outerWidth(), outerHeight() has a chance to give an error.

I understood `non-integer values` is may return floating, not "NaN"

#2809427: Update jQuery UI to 1.12 landed!

#42 still applies fine.
Re-uploading it here to make it clear what to work on (and see what the bot says) =)

Ok, so in the layout-builder work (we're getting a patch together) we have a Drag and Drop layout utility which leverages jQuery UI to move blocks around on the page. This is great, but with core's current version, it returns inaccurate widths which causes draggables to move back to the left-most pixel baseline when dragging if they are a fractional width (imagine a block in a second column jumping over to the first column as soon as you try to drag it). We looked into solving this and decided we'd have to rewrite jQuery's width handling so rather than do that, we looked into updating to a newer version of jQuery since the width handling appeared to have been rewritten. Upon doing so, our problem went away (YAY) however, this patch breaks Settings Tray (which we are also using extensively).

Post application of this patch, settings tray appears on the left side of the screen instead of the right, its title and close buttons are hidden under the administrative menu, and it doesn't stay affixed to the side when you scroll. Probably need to solve all those things before we commit this patch.

Eclipse

Moving this back to needs review.

- There is #2894283: Improve Toolbar Height calculation to accommodate jQuery 3.0 changes. which will fix the odd height calculation behavior present in jQuery 3.0
- @EclipseGc, I tested the patch from #63 against ba8156144b and didn't see the described behavior.

Didn't see anything weird with outside in either. Tabledrag with keyboard is broken though.

@nod_ Can you open an issue and I can take a look? Thanks 😀

Tabledrag with keyboard is broken though.

It is in HEAD as well. See #2725259: [regression] Table Drag handles no longer respond to up/down arrow keys. Looks to me like the patch in that issue fixes it equally both for HEAD (jQuery 2.2.4) and with #63 (jQuery 3.2.1) applied. So I don't think that needs to hold up this patch, unless someone is seeing an additional regression introduced by the jQuery upgrade.

Tour is broken (Tested: Language)

Is it still? I applied #63, installed Language module, went to /admin/config/regional/language, and the tour button appears in my toolbar, and when I click it, it gives me a tour of that page. What's broken about it?

1. +++ b/core/assets/vendor/jquery/jquery.min.map
   @@ -1 +1 @@
   -{"version":3,"sources":["jquery.js"],"names":["global","factory",...
   +{"version":3,"sources":["jquery-3.2.1.js"],"names":["global","factory",...
   

   Is it a problem that this references jquery-3.2.1.js, but we don't have that file? In our repo, it's named just jquery.js.

2. +++ b/core/core.libraries.yml
   @@ -341,10 +341,10 @@ html5shiv:
   -    url: https://github.com/jquery/jquery/blob/2.2.4/LICENSE.txt
   +    url: https://raw.githubusercontent.com/jquery/jquery/3.2.1/LICENSE.txt
   

   Is there something that requires us to switch the host name like this? Could we just bump the version number in this patch and have a separate issue for moving the license links from github.com to raw.githubusercontent.com?

Since this is a major version upgrade of a library, with known BC breaks, I think a change record would be good. Could someone write one? It could potentially be as minimal as just pointing contrib developers to http://jquery.com/upgrade-guide/3.0/. However, I think information from #55 and/or other tips we can offer contrib developers to watch out for, would be appreciated.

Once #70 and #71 are addressed, my inclination is to just commit this in the next day or so, so that:

1. We have a few days before alpha in which to discover if there's something really bad that needs fixing and/or this reverted before alpha.
2. People testing 8.4-alpha1 could help us find miscellaneous things that need fixing, and we can get those fixed by beta.

Any objections to that?

Not sure about the impact of #70.1

Addressing #70.2.

Re #72: +1

Re #70.2: just opened #2897837: Change the links on core.libraries.yml licenses so they link to the licenses themselves

I queued #63 for a retest to see if it has the same failure as #73.

Not sure about the impact of #70.1

I'm pretty sure we need to change it to match our actual source filename. See #2485575-20: Update jQuery to 2.1.4 and the comments below that.

I queued #63 for a retest to see if it has the same failure as #73.

Indeed it does. So the failure is not an artifact of the reroll, but reflects some change in HEAD that has occurred since #63 was initially posted.

Ah, thanks @effulgentsia for the explanation, doing #70.1 now.

The interdiff is not going to be very useful, but here are the two things I changed:

• "sources":["jquery-3.2.1.js" -> "sources":["jquery.js"
• "file":"jquery-3.2.1.min.js" -> "file":"jquery.min.js"

Is it still? I applied #63, installed Language module, went to /admin/config/regional/language, and the tour button appears in my toolbar, and when I click it, it gives me a tour of that page. What's broken about it?

It should open in the centre and have gray overlay as BG.

For the test failures in #79, I opened a child issue: #2898261: In jQuery 3, $('#') throws syntax errors. Help with fixing that would be most appreciated!

Combined patch with #2898261: In jQuery 3, $('#') throws syntax errors

Adding @droplet to issue credits (who wrote the patch over there).

Will remove myself straight after, as only uploading combined patch.

I opened #2898808: [regression] With jQuery3, modal tour tips lose their grey background and centering for #81. If that can be addressed before this issue gets committed, that would be great. But I'm also fine with not blocking this on that, and leaving it for a follow-up.

I drafted a small change record in https://www.drupal.org/node/2898818. If anyone wants to expand it for #71, go for it.

Updated combined patch. This combines this issue's #79 with #2898261-16: In jQuery 3, $('#') throws syntax errors.

Per #72, I still think this is worth getting into the alpha if possible. I do have some concerns that jQuery BC breaks like the one in #2898261: In jQuery 3, $('#') throws syntax errors may still be lurking in parts of core that don't have JS test coverage. And because of that, I feel that if this is to make it into alpha at all, it really needs to go in before the freeze, so there's the full freeze window in which to discover a regression bad enough that requires a fix or a revert.

Once #2898261: In jQuery 3, $('#') throws syntax errors lands, the patch in #79 is the one to commit. It looks great to me, and I believe that every comment in this issue for problems discovered has been addressed or has a non-blocking child/related issue open for. Therefore, RTBC, and just as in #2898261-21: In jQuery 3, $('#') throws syntax errors, leaving it to committers in other timezones to make the call on whether to commit this prior to the freeze or not.

Thanks all!

preemptive tag with 8.4.0 release notes

updating issue credits to Wim Leers for manual testing, reviews, mentoring etc

embarking on a big round of manual testing

Manual testing results on Firefox Nightly 56.0a1 on Linux

Works
- states (tested in installer)
- password strength indicator/match (installer)
- toolbar
- node add/edit forms ckeditor, details element, autocomplete
- contextual links (node page, front page)
- views UI (dialogs, auto preview, preview)
- taxonomy term overview (tabledrag, drop buttons)
- type ahead table filter (simpletest UI, admin/modules)
- filter formats editing (updates allowed html when adding ckeditor buttons)
- drag ordering on filter plugins (filter formats edit)
- placing a block (dialog, highlighting the updated row)
- field ui manage display (changing widgets, formatters, visibility, ajax update of settings)

Doesn't work
- tours - existing issue already opened - I agree this can be a follow up.
- settings tray - at first I thought this was horribly broken, but then I span up a HEAD install and the same issues exist. So putting that down to experimental status - e.g. Go to home page, click 'edit' button to show contextual links. Leave it enabled. Visit modules page and enable settings tray. Everything is broken. Navigate (via url because links don't work) to homepage. Turn off edit and then you can go back to admin pages again. Once you get out of that rabbit hole, things seem to work - although clicking 'advanced block options' when editing a block gets you back into the same rabbit hole (again exists in HEAD).

Adding myself to issue credits for this manual testing (at least an hour).

I agree with @effulgentsia in #89, let's get this into the alpha so we can expose bugs (many eyes make shallow bugs).

This needs a reroll because #2898261: In jQuery 3, $('#') throws syntax errors went into 8.4.x (still needs to go into 8.5.x).

We need the uncombined patch. Is #79 the one?

Yes, #79 is the one. Here's a reupload of it.

The failure in the 8.5.x test for #96 is due to that test having started prior to #2898261-31: In jQuery 3, $('#') throws syntax errors being pushed to 8.5.x. I queued a retest. Optimistically setting back to RTBC.

I double-checked with nod_ and drpal on IRC for signoff on this one. Thank you everyone—the tedious manual testing is appreciated!

I'll publish the change record. Thanks!

• Committed 2983a0d and pushed to 8.5.x.
• Committed ce16619 and pushed to 8.4.x.

Next step is #2899141: jQuery Form Plugin update to latest stable release because current core version may have incompatibilities according https://github.com/jquery-form/form/releases/tag/v4.2.0

@xjm and I are concerned by the fact that #2898808: [regression] With jQuery3, modal tour tips lose their grey background and centering impacts Tour's usability more than we understood at the time this was committed. Consequently, we have decided to revert this issue if #2898808 is not fixed before 8.4.0-beta1. So, please help out on #2898808! Thank you, as always.

I am cross-posting this.

Hi, now that #2898808 has been closed, would this issue be solved in 8.4?

It is in 8.4.x and it is fixed.

Thank you!