Make WordPress Core

#57946 closed defect (bug) (fixed)

Deprecated jQuery in Farbtastic

Reported by: malae's profile Malae Owned by: audrasjb's profile audrasjb
Milestone: 6.3 Priority: normal
Severity: normal Version: 6.1.1
Component: External Libraries Keywords: has-patch commit
Focuses: javascript Cc:

Description (last modified by sabernhardt)

jQuery.fn.mousedown() event shorthand is deprecated in /wp-admin/js/farbtastic.js
jQuery.fn.bind() is deprecated in /wp-admin/js/farbtastic.js

Attachments (2)

57946.patch (16.4 KB) - added by mrinal013 12 months ago.
Replacement of the old fabrasic.js
57946.1.patch (2.0 KB) - added by sabernhardt 11 months ago.
replace old jQuery methods and add documentation notes

Download all attachments as: .zip

Change History (25)

#1 @sabernhardt
15 months ago

  • Component changed from Shortcodes to External Libraries
  • Description modified (diff)
  • Focuses javascript added
  • Summary changed from Deprecated jQuery to Deprecated jQuery in Farbtastic

Thanks for the report!

Usually, external library issues should be reported upstream, but the Farbtastic GitHub repo does not have recent activity.

This script was moved to the js folder in [43309], though the most recent content change was [16305].

#2 @SergeyBiryukov
15 months ago

  • Milestone changed from Awaiting Review to 6.3

The upstream repository has been archived by the owner on Feb 27, 2019, and is now read-only.

Looks like this will now be an "adopted" library, and can be patched with necessary changes.

#3 @Presskopp
14 months ago

Farbtastic itself is long dead. But it still is to find in the Twenty Eleven Theme and some remnants are still to find in Core. Instead of patching it, we should completely get rid of it, I'd say.

This ticket was mentioned in PR #4626 on WordPress/wordpress-develop by @mrinal013.


12 months ago
#4

  • Keywords has-patch added

Trac ticket: 57946

#5 @peterwilsoncc
12 months ago

I don't think it's possible to remove the library as it's used by many plugins (a few false positives in there) and quite a few themes.

One of the true positives is CMB2 which is used by many more sites than the install count suggests as it's often included as a package within client project themes and/or plugins.

As the library is already considered adopted by core, it's fine to modify the files in the pull request.

#6 @oglekler
12 months ago

  • Milestone changed from 6.3 to 6.4

If we cannot remove this library, we need to adopt it and this ticket needs a new patch. Due to this, I am moving this into the 6.4 milestone.

#7 @mrinal013
12 months ago

  • Keywords needs-testing added

@peterwilsoncc
Just change some code of the core Farbtastic.js library. So further testing is required.

Last edited 12 months ago by mrinal013 (previous) (diff)

#8 @oglekler
12 months ago

  • Milestone changed from 6.4 to 6.3

Because patch is now ready for testing and this ticket was in 6.3 milestone, I am returning it to this milestone.

@mrinal013 commented on PR #4626:


12 months ago
#9

Reopen PR

@mrinal013 commented on PR #4626:


12 months ago
#10

Reopen PR

@mrinal013 commented on PR #4626:


12 months ago
#11

Reopen PR

#13 @mrinal013
12 months ago

  • Keywords reporter-feedback needs-dev-note added

This ticket was mentioned in Slack in #core-test by mrinal. View the logs.


12 months ago

#15 @mrinal013
12 months ago

  • Keywords reporter-feedback needs-dev-note removed

Removed need-dev-note and reporter-feedback. Because I think they are not appropriate now.

@mrinal013
12 months ago

Replacement of the old fabrasic.js

@sabernhardt
11 months ago

replace old jQuery methods and add documentation notes

#16 follow-up: @sabernhardt
11 months ago

Note: I did not test the patch. I simply reduced the changes, mainly so it's easier to review.

#17 in reply to: ↑ 16 @mrinal013
11 months ago

Replying to sabernhardt:

Note: I did not test the patch. I simply reduced the changes, mainly so it's easier to review.

Thanks.

In https://core.trac.wordpress.org/attachment/ticket/57946/57946.patch, I did somehow similar changes as https://core.trac.wordpress.org/attachment/ticket/57946/57946.1.patch. Additionally, in https://core.trac.wordpress.org/attachment/ticket/57946/57946.patch,

  1. Following coding standard, added with wp-prettier.
  2. Use navigator.userAgent instead of navigator.appVersion, since navigator.appVersion is not recommended https://developer.mozilla.org/en-US/docs/Web/API/Navigator/appVersion.

#18 follow-up: @jorbin
11 months ago

Following coding standard, added with wp-prettier.

This adds alot of unnecessary changes which are harder to review and also mean that if fabrasic ever comes back from the dead, it would really hard to upstream these changes.

#19 in reply to: ↑ 18 @mrinal013
11 months ago

Replying to jorbin:

Following coding standard, added with wp-prettier.

This adds alot of unnecessary changes which are harder to review and also mean that if fabrasic ever comes back from the dead, it would really hard to upstream these changes.

Like this idea.

Also, tested the patch: https://core.trac.wordpress.org/attachment/ticket/57946/57946.1.patch

It works ok on my end.

Last edited 11 months ago by mrinal013 (previous) (diff)

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


11 months ago

#21 @audrasjb
11 months ago

  • Keywords commit added; needs-testing removed

self assigning for commit

#22 @sabernhardt
11 months ago

If navigator.appVersion should (ever) be removed, too, I think it could be better to remove the full IE6 PNG background section.

#23 @audrasjb
11 months ago

  • Owner set to audrasjb
  • Resolution set to fixed
  • Status changed from new to closed

In 56252:

External Libraries: Update deprecated jQuery code in Farbtastic lib.

This changeset updates some deprecated jQuery code in the Farbtastic external library. As this vendor script is not maintained anymore, this changeset
also adds a docblock to specify that the library has been "adopted" by WP Core.

Props Malae, sabernhardt, SergeyBiryukov, Presskopp, mrinal013, peterwilsoncc, oglekler, jorbin.
Fixes #57946.

Note: See TracTickets for help on using tickets.