Make WordPress Core

Opened 13 months ago

Closed 9 months ago

#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 9 months ago.
Replacement of the old fabrasic.js
57946.1.patch (2.0 KB) - added by sabernhardt 9 months ago.
replace old jQuery methods and add documentation notes

Download all attachments as: .zip

Change History (25)

#1 @sabernhardt
13 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
13 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
12 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.


10 months ago
#4

  • Keywords has-patch added

Trac ticket: 57946

#5 @peterwilsoncc
10 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
10 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
10 months ago

  • Keywords needs-testing added

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

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

#8 @oglekler
10 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:


10 months ago
#9

Reopen PR

@mrinal013 commented on PR #4626:


10 months ago
#10

Reopen PR

@mrinal013 commented on PR #4626:


10 months ago
#11

Reopen PR

#13 @mrinal013
10 months ago

  • Keywords reporter-feedback needs-dev-note added

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


10 months ago

#15 @mrinal013
10 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
9 months ago

Replacement of the old fabrasic.js

@sabernhardt
9 months ago

replace old jQuery methods and add documentation notes

#16 follow-up: @sabernhardt
9 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
9 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
9 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
9 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 9 months ago by mrinal013 (previous) (diff)

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


9 months ago

#21 @audrasjb
9 months ago

  • Keywords commit added; needs-testing removed

self assigning for commit

#22 @sabernhardt
9 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
9 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.