s9y / Serendipity

A PHP blog software
https://s9y.org
BSD 3-Clause "New" or "Revised" License
203 stars 86 forks source link

update bundled jQuery to 3.x #686

Open mmitch opened 4 years ago

mmitch commented 4 years ago

I've just found the Google Lighthouse plugin for Firefox and ran it against my blog which is already updated to Serendipity 2.3.4. The Lighthouse report mentions 2 medium security issues in the jQuery 1.12.4 that is bundled with Serendipity 2.3.4: the two red lines at https://snyk.io/vuln/npm:jquery?lh=1.12.4

As far as I can tell:

I don't know much about JavaScript or jQuery – is an update or version switch relatively painless or would this mandate a rewrite of half of Serendipity's frontend?

(According to the contribution guidelines, I have contacted Garvin by email to check whether this is security relevant – his quick triage/response was: 1. the bugs in jQuery are more on the cosmetic side 2. updating jQuery might break plugins 3. I should go on to post this issue here on Github: [x] done :-)

yellowled commented 4 years ago

is an update or version switch relatively painless or would this mandate a rewrite of half of Serendipity's frontend?

I have to admit I haven't updated anything from s9y 1.x to newer versions in a long time. The issue is not so much s9y's frontend, but plugins (as Garvin mentions). Which, of course, is a matter of testing first and foremost.

The most likely issue is probably third-party stuff breaking. Some outdated, unmaintained script that one of our plugins or themes uses. I'll try to gather information on which themes/plugins even use jQuery. (I don't think dropping jQuery altogether is an option.)

yellowled commented 4 years ago

Core themes that use jQuery

The biggest potential issue I have seen here so far is a dependency on FitVids, which requires jQuery, in clean-blog, timeline and 2k11, which is a third-party script that hasn't been updated in a long time. Then there's of course our backend JS, which uses jQuery and also uses plugins that depend on jQuery.

It seems like no core plugin uses jQuery.

Additional plugins that require jQuery

I have not yet looked at what they do, that's just the result of a quick grep.

yellowled commented 4 years ago

I have prepared upgrading to jQuery 3.4.1 and adding the migration plugin in a branch (see PR above). I don't know when I will be able to test this myself. If someone else can test this earlier, go right ahead of course.

mmitch commented 4 years ago

I've manually applied the PR to my productive Serendipity 2.3.4 blog and don't see any issues.

The JavaScript console shows only this and now further warnings on any page tested:

JQMIGRATE: jQuery 3.0.0+ REQUIRED                        jquery-migrate.js:2:561
JQMIGRATE: Migrate is installed, version 3.1.0           jquery-migrate.js:2:696

The Lighthouse security vulnerability test is now passed.

My test contains:

I encountered some mixed content warnings in the admin interface and the spamblock bayes "empty trashcan" only seems to work on the current page, not all entries, but both of these things are propably unrelated to the jQuery upgrade.

I think I'll just let it run with the patch applied for now. If anybody wants to have a look: https://www.cgarbs.de/blog/

yellowled commented 4 years ago

Seems like I hit jackpot on the first extra plugin I tried. 😂

Using serendipity_event_commentedit with jQuery 3.5.0, jQuery migrate and (maybe that's the error?) PHP 7.3 gives me an empty page with a console error

GET https://s9y.ddev.site/archives/2-Big-Buck-Bunny.html net::ERR_CONTENT_DECODING_FAILED 200

@onli can you make any sense of this? Could commentedit be incompatible with newer jQuery versions?

yellowled commented 4 years ago

Well, to sum this up, I don't think we can currently merge this since it breaks some plugins. Plus, testing it revealed some potential issues within other plugins that prevented me from testing at all. On the plus side, I think most of these only use jQuery in the backend.

surrim commented 3 years ago
  • serendipity_event_osm 🚫 (see #721)

This plugin don't need jQuery. jQuery is used for the dropdown description texts in the backend.