Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#50093 closed defect (bug) (fixed)

Remove the wp.a11y jQuery version file

Reported by: afercia's profile afercia Owned by: ocean90's profile ocean90
Milestone: 5.7 Priority: normal
Severity: normal Version: 5.3
Component: Administration Keywords: has-patch early commit
Focuses: javascript Cc:

Description (last modified by SergeyBiryukov)

The jQuery version of wp-a11y is no longer used since [46179].

To my understanding, this old script is no longer registered (replaced by the new version from the packages) but the physical file is still there at src/js/_enqueues/wp/a11y.js. Not sure why the physical file was kept. I guess it can be removed safely?

Attachments (2)

50093.diff (31.6 KB) - added by audrasjb 5 years ago.
Remove a11y/js from JS dependencies
50093.1.diff (2.9 KB) - added by audrasjb 5 years ago.
Scripts/loader: remove a11y.js dependancy since it is now loaded in the new a11y package.

Download all attachments as: .zip

Change History (21)

@audrasjb
5 years ago

Remove a11y/js from JS dependencies

#1 @audrasjb
5 years ago

  • Keywords has-patch dev-feedback 2nd-opinion added

Hi,

In 50093.diff, I tried to remove this dependencies (I also updated the removed files function and some inline docs).

However, it looks like it will eventually cause some unit tests to fail:
https://core.trac.wordpress.org/browser/trunk/tests/phpunit/tests/dependencies/scripts.php#L729

Should I replace wp-a11y deps with another random one in this unit test?

#2 @afercia
5 years ago

@audrasjb the various dependencies are used by the new a11y package. That part shouldn't be touched :) I think only the old physical file needs to be removed.

#3 @audrasjb
5 years ago

  • Keywords needs-refresh added; dev-feedback 2nd-opinion removed

Ahhh… 🤦‍♂️ ok, got it.

@audrasjb
5 years ago

Scripts/loader: remove a11y.js dependancy since it is now loaded in the new a11y package.

#4 @audrasjb
5 years ago

  • Keywords needs-refresh removed

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


5 years ago

#6 @youknowriad
5 years ago

I've been told that we can remove files from Core for backward compatibility as plugins could reference them directly. I'd be personally in favor of removing and if it's something we're allowed to do, I think we should consider simplifyng the placement of all the JS files in Core later.

#7 @davidbaumwald
5 years ago

@audrasjb @afercia, is this still something that is doable in 5.5 with the time remaining?

#8 @afercia
5 years ago

  • Milestone changed from 5.5 to 5.6

I'd say it's safer to to at the beginning of the next release cycle.

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


5 years ago

#10 @helen
4 years ago

  • Keywords early added
  • Milestone changed from 5.6 to Future Release

Hmm another ticket that hasn't been modified in quite a while, I thought that we had scrubbed through those recently. Punting and marking as early.

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


4 years ago

#12 @hellofromTonya
4 years ago

  • Milestone changed from Future Release to 5.7

Moving into the 5.7 milestone per scrub today and Helen's note above.

#13 @hellofromTonya
4 years ago

  • Patch applies cleanly against trunk.
  • When running tests npm run test:php, tests pass as expected.

@audrasjb is the patch ready for commit?

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


4 years ago

#15 @SergeyBiryukov
4 years ago

  • Description modified (diff)

#16 @SergeyBiryukov
4 years ago

  • Keywords commit added

50093.1.diff seems good to go.

#17 follow-up: @Hareesh Pillai
4 years ago

Should we also update the $_old_files array in the update-core.php file?

#18 @ocean90
4 years ago

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

In 49972:

Script Loader: Remove unused source file for wp-a11y.js which was replaced with the @wordpress/a11y package.

Props afercia, audrasjb.
See [46167], [46169], and [46179].
Fixes #50093.

#19 in reply to: ↑ 17 @SergeyBiryukov
4 years ago

Replying to Hareesh Pillai:

Should we also update the $_old_files array in the update-core.php file?

Thanks for mentioning that, it appears to be already done in [46167].

Note: See TracTickets for help on using tickets.