GibbonEdu / core

Gibbon is a flexible, open source school management platform designed to make life better for teachers, students, parents and leaders.
https://gibbonedu.org
GNU General Public License v3.0
460 stars 299 forks source link

Updated JQuery and JQuery Migrate Files #1775

Closed felixOlesen closed 7 months ago

felixOlesen commented 7 months ago

Description Updated: JQuery: v2.2.4 to v3.7.1
JQuery Migrate: v1.4.1 to v3.4.0

The new JQuery versions were copied into their corresponding JQuery files located at: gibbon/lib/jquery/jquery-migrate.min.js gibbon/lib/jquery/jquery.js

Motivation and Context This update was prompted by certain vulnerabilities being pointed out to us, which was a result of an outdated version of JQuery and JQuery Migrate.

How Has This Been Tested? Tested locally with these features in mind:

NOTE: These are features mentioned in the core.js file that were not able to be tested due to an unrelated bug in my development environment:

yookoala commented 7 months ago

To facilitate discussion of the upgrade. Here are some context.

Browser Support

Some information about the browser support of the latest jQuery v3 library.

yookoala commented 7 months ago

Compatibility with Other Javascript Libraries

Version compatibility checks for all libraries in GibbonCore's lib folder (as of 9f92840):

Library Current Version jQuery Compatibilty Potential Solution Status
ace v1.4.12 (Do not depend on jQuery) -
chained v1.0.1 (modified) no information Need further study
chart.js v3.6.0 (Do not seem to depend on jQuery) -
instascan (unknown) (Do not seem to depend on jQuery) -
jquery-autosize v3.0.15 no information Need further study
jquery-form v3.51.0-2014.06.20 only partially compatible with v3 Need further study
jquery-jslatex v1.2 no information Need further study
jquery-timepicker v1.11.10 no information Need further study
jquery-tokeninput v1.6.2 no information Need further study
jquery-ui v1.11.4 Seems to require v1.5 or above to be compatible with jQuery v3 Upgrade to v1.5+ ❓ Need work
LiveValidataion v1.3 (Do not seem to depend on jQuery) -
thickbox v3.0 / v3.1 no information Need further study
tinymce v5.6.2 Some reported issue with jQuery 3.5.1 Need further study
vis v4.21.0 (Do not seem to depend on jQuery) -
yookoala commented 7 months ago

My conclusion:

  1. Both jQuery v2 and v3 support IE9+, which is probably the major concern in backward compatibility.
  2. jQuery v3 does have some breaking changes, which might affect any script that is using it (see the summary of important changes in v3).
    • Need to upgrade some of the existing libraries (e.g. jquery-ui).
    • Need to explore to update / remove some of the libraries (e.g. jquery-form, tinymce).
    • Need to study if some of the libraries have issue with jQuery 3 (e.g. chained, jquery-autosize, jquery-jslatex, jquery-timepicker, jquery-tokeninput, thickbox) and decide what to do with them.

Follow up checklist:

SKuipers commented 7 months ago

Thanks @yookoala, this is incredibly thorough and useful information, much appreciated. TinyMCE is used in many different places, so definitely and important one to not break.

@felixOlesen Do you want to have a look at the version compatibility information Koala has put together and see if you can update the updateable libraries, and test the remaining ones. Based on the checklist, here are some pointers to where these are being used:

felixOlesen commented 7 months ago

Thanks @SKuipers and @yookoala for your incredibly detailed help. This is really helpful for me who still has a lot to learn about Gibbon's ins and outs.

I'll get myself more familiar with these libraries and compatibilities with different versioning and follow up with the bullet points you've mentioned @SKuipers and see how everything is working with regards to the upgrade.

yookoala commented 7 months ago

@felixOlesen: There is a merge conflict with the v27.0.00 branch now. You'd probably want to do a git rebase on the latest v27.0.00 commit before carrying on your. work.

felixOlesen commented 7 months ago

Just adding a useful link on jQuery Migrate warnings

felixOlesen commented 7 months ago

Here is what I've been able to find and test so far. The most worrying issue is with Thickbox, where the pop-ups in gibbon get stuck on the loading bar. Another issue of note is with TinyMCE which displays warnings caught by jQuery Migrate. Most of these plugins haven't seen any updates in 2 to 6 years on average so it's difficult to find any more compatibility information.

Screenshot 2024-01-12 at 10 56 21 Screenshot 2024-01-12 at 10 33 36
felixOlesen commented 7 months ago

With the most of the libraries working as expected, I believe the next step would be to debug the current problems with Thickbox and TinyMCE to see where it corresponds to specific updates in the plugins or jQuery itself.

After doing that, would it be the best course of action to find a good middle-ground between jQuery and plugin versions and then applying those to maintain full functionality?

yookoala commented 7 months ago

Maybe we can replace thickbox with something similar that works with newer jQuery?

Some suggestions:

  1. Venobox Pros: Like thickbox, another variant of Lightbox for displaying images. Support responsive. Cons: Can be used with / without jQuery. Doesn't specify browser support. Need to check on IE9-IE11.

  2. <dialog> element with Dialog-Polyfill Pros: Closer to HTML5 standard. More flexibility and control to theme. Can be used with / without jQuery. The polyfill library declared compatability with IE9+. Cons: More works to begin with (because doing custom code.

SKuipers commented 7 months ago

Hi @yookoala In the end, we were able to get Thickbox working by making one small edit to the thickbox-compressed.js file, and we were able to mute the deprecation messages from jQuery migrate for now, confirming that Tiny MCE is working as expected. Long term, our aim will be to remove jQuery entirely and replace it with something more modern, but in the meantime it looks like these fixes have addressed all the short-term jQuery compatibility issues, and we're ready to merge this PR into the core.

yookoala commented 7 months ago

Getting rid of jQuery would be nice, but probably need lots of polyfill libaries to make things work on Internet Explorer.