Replace wp-a11y.js with @wordpress/a11y package

[omarreiss]

We've extracted wp-includes/wp-a11y.js to a reusable package ​wordpress/a11y which is published on NPM. Let's make sure this package is also used in WordPress core. Once all the JavaScript is built using webpack, we can also import this package wherever it is used and configure webpack to load is an external.

[herregroen]

Added a patch that fully removes the original registration including the file.

As the wp-a11y handle is already registered no further action is required. Some PHPUnit tests required minor modifications in order to ensure wp-a11y is registered and to account for it's dependencies.

[afercia]

Seems to me everything works as expected, from an accessibility perspective. Thanks!

[omarreiss]

Signing off for commit. @gziolo, @herregroen or @atimmer. Feel free.

[ocean90]

Before it gets committed it'd be good to fix the indentation in update-core.php and update the patch for [43877].

[gziolo]

expected  = "<script type='text/javascript' src='/wp-admin/load-scripts.php?c=0&amp;load%5B%5D=jquery-core,jquery-migrate,wp-polyfill-ecmascript,wp-dom-ready,wp-a11y&amp;ver={$ver}'></script>\n";

Should wp-polyfill-ecmascript be updated to wp-polyfill?

[desrosj]

In 45066.3.diff​:

• Fixed indentation in update-core.php.
• Changed wp-polyfill-ecmascript to wp-polyfill.
• Fixes failing tests due to wp_get_script_polyfill() now being triggered by wp-polyfill.

Also, wp_default_packages_vendor() and wp_default_packages_scripts() were already being called in the 5.0 branch. Since wp_default_packages() calls these, I removed those calls.

[ocean90]

@desrosj Using wp_default_packages() will cause a fatal error. There’s no need to switch to that, see #45271 for background.

[afercia]

Seems to me the first part of the big $_old_files array has a wrong indentation compared to 4.9:

https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/update-core.php?rev=42777#L20

https://core.trac.wordpress.org/browser/branches/5.0/src/wp-admin/includes/update-core.php#L20

[herregroen]

@afercia Seems that way.

Just updated the patch to correct that indentation.

I've also changed the expected test output to unminified files. wp-tests-config-sample.php sets the ABSPATH as the src directory in which case unminified JS sources are used.

[herregroen]

Or it might make more sense to indent everything as it was. Not sure why the change in the 5.0 branch was made.

[ocean90]

Let's keep the indentation as in 45066.4.diff​ to keep the diff small. Trunk has the code style fixes applied which the 5.0 branch has not since it was branched from 4.9.

[herregroen]

@ocean90 Latest patch updated with both the adapted tests and smallest changes in regards to update-core.php.

Testing everything shows no issues so the latest patch should be good for commit.

[omarreiss]

Hmm, I signed off 11 days ago. Now it seems late in the process to commit this nice to have. Punting to 5.0.1. This only removes some duplication after all.

[desrosj]

Talked this through with @herregroen and wanted to put this here for visibility. Locally, the tests in the most recent patch fail for me. But it is a .min vs no .min issue. My site was provisioned in VVV using trunk, which has the following conditional:

if ( defined( 'WP_RUN_CORE_TESTS' ) && WP_RUN_CORE_TESTS ) {
	define( 'ABSPATH', dirname( __FILE__ ) . '/build/' );
} else {
	define( 'ABSPATH', dirname( __FILE__ ) . '/src/' );
}

WP_RUN_CORE_TESTS is defined in phpunit.xml.dist as 1 and would cause the tests to return the minified versions.

When this gets ported over to trunk, the tests may need to change to look for the minified versions.

[desrosj]

Patch is no longer applying cleanly for me. Going to punt this to 5.1 because it is also still in need of testing.

[desrosj]

This still needs a refresh. Punting to 5.2.

@herregroen If you able to refresh before 5.1 Beta1 tomorrow we can look at including it.

[herregroen]

@desrosj going to need some more time for this I'm afraid.

wp-polyfill is now a dependency of the packaged wp-a11y which means that during the unit tests the full polyfill browser tests are printed out which cause two tests to fail. The packaged scripts also contain various version strings that are package versions which I'm not sure are exposed.

Will need a bit of time to write an elegant way to unit test these as I don't think removing wp-a11y altogether from unit tests would be good. Having an elegant solution for that means we can also start adding unit tests for the other packages which right now aren't included at all in tests/phpunit/tests/dependencies/scripts.php I believe.

[desrosj]

This ticket has not received any attention during the 5.2 cycle. With beta 1 tomorrow, going to punt this to 5.3.

[sstoqnov]

Replace wp-a11y.js with @wordpress/a11y & fix failing tests

[sstoqnov]

Remove wp-a11y from build process

[sstoqnov]

Hi,

I've used the previous patches and in 45066.8 the failing tests are fixed.

I was not sure if it ok to remove the wp-a11y from Gruntfile.js, so I've attached another patch 45066.9, where it's removed.

[audrasjb]

Hello, we looked into this ticket during accessibility bug scrub.
Any update on this ticket? As a reminder, 5.3 beta 1 is on September 23. Do we need more work/test before commit?

[desrosj]

Thanks for the refresh @sstoqnov!

I applied this patch and did some testing. The tests pass now. However, when I use the admin area, View Details for themes is broken, and there are several console errors that I am seeing throughout the admin area.

[sstoqnov]

Hey @desrosj

Thanks for the feedback.
I just applied the patch to the latest trunk code, but the "Theme details" screen looks ok and I don't see any errors in the console.

Would it be possible to provide more information about those errors please?

[afercia]

To my understanding 45066.8.diff​ and 45066.9.diff​ need to be applied together, correct?

45066.8.diff​ doesn't apply cleanly for me, using grunt patch.

[sstoqnov]

Correct both patches have to be merged.

[whyisjake]

combined diff

[whyisjake]

Hotfix for the failing tests.

[SergeyBiryukov]

[46167], for reference (missed the ticket):

Accessibility: Replace wp-a11y.js with @wordpress/a11y package

We've extracted wp-includes/wp-a11y.js to a reusable package wordpress/a11y which is published on NPM. Let's make sure this package is also used in WordPress core. Once all the JavaScript is built using webpack, we can also import this package wherever it is used and configure webpack to load is an external.

Props omarreiss, herregroen, desrosj, ocean90, afercia, sstoqnov

[whyisjake]

In 46169:

Accesibility: Replace wp-a11y.js with @wordpress/a11y package

Continuation of [46167]. This fixes the tests that ended up broken following the previous commit.

We've extracted wp-includes/wp-a11y.js to a reusable package wordpress/a11y which is published on NPM. Let's make sure this package is also
used in WordPress core. Once all the JavaScript is built using webpack, we can also import this package wherever it is used and configure
webpack to load is an external.

Props, garrett-eclipse

Fixes #45066

[garrett-eclipse]

@whyisjake I believe this is resulting in plugins not able to install see #48071

[afercia]

I think $scripts->add( 'wp-a11y' ... needs to be removed from script-loader.php and after that npm install && grunt build? Otherwise, the old wp-a11y is still enqueued and gives a 404 in many pages (not only plugins).

[afercia]

In 46179:

Script Loader: Remove old wp-a11y from the loaded scripts after [46167] and [46169].

Props garrett-eclipse, sstoqnov.
See #45066.
Fixes #48071.

[SergeyBiryukov]

In 46180:

Tests: Update unit tests to account for the removal of old wp-a11y from the loaded scripts in [46179].

Props sstoqnov.
See #45066, #48071.

[SergeyBiryukov]

In 46181:

Docs: Format multi-line comment in [46180] per the documentation standards.

See #45066.

[whyisjake]

Thanks y'all for the fix!

[Hareesh Pillai]

Although all the QUnit tests are passing, I observe the following console error when running the unit tests.
Uncaught TypeError: external_this_wp_domReady_default(...) is not a function on line 208 of the a11y.js file.

[desrosj]

Reopening to investigate the TypeError above.

[ocean90]

In 46388:

QUnit: Include script dependencies for wp-a11y script to fix a TypeError.

Also, add missing message argument to Customizer notifications to ensure tests are passing.

Fixes #45066.