feat: bundle the a11ychecker plugin

[julien]

Test plan:

• Run ./ck.sh build
• Use the custom build in liferay-portal
• Enable the a11ychecker plugin
• See it working

Notes:

• Until #85 is merged you will get an error about missing CSS files from the
  balloonpanel plugin but it won't prevent the a11ychecker plugin to
  work - the styles will just be a bit broken)
• This will create a global jQuery object: a11ychecker (and also
  ./ckeditor/adapters/jquery.js) expect window.jQuery to be defined
  but we can improve that later on if everyone agrees

[wincent]

• This will create a global jQuery object: a11ychecker (and also ./ckeditor/adapters/jquery.js) expect window.jQuery to be defined but we can improve that later on if everyone agrees.

I don't think we should ship anything that defines jQuery on the window, because if people discover jQuery on the window, they may come to depend on it. That runs counter to what we did when we turned off frontend-js-jquery-web by default (where we want people to get it only if they explicitly turn it on). And speaking of frontend-js-jquery-web, having two places that compete to add a global jQuery instance (ie. this editor package, and that OSGi module), will be yet another point where we'll need to keep versions synchronized, and we might open ourselves up to races depending on load order and internal state that might exist inside the two jQuery modules.

In other words, the question is, is there a way to configure a11ychecker to make use of a specific jQuery instance that we provide in a non-global way? I would be fine with assigning to window if it is done in a suitably "private" manner (eg. window.__LIFERAY_CKEDITOR_INTERNAL_$ or something similarly "private" looking).

About the adapter, I am not sure if it is active by default. git grep adapters/jquery and git grep '\bckeditor\(' don't show any obvious explicit use in liferay-portal. In a running portal instance looking at a CKEditor editors, I see a $ global already (sigh... where is that coming from? I don't think it is AUI, because AUI.$ is unset.), and no $().ckeditor() function, which suggests the adapter is not automatically active. I think it might just be sitting there dormant inside CKEditor and won't have any effect unless loaded/used.

[mateomustapic]

• This will create a global jQuery object: a11ychecker (and also ./ckeditor/adapters/jquery.js) expect window.jQuery to be defined but we can improve that later on if everyone agrees.

I don't think we should ship anything that defines jQuery on the window, because if people discover jQuery on the window, they may come to depend on it. That runs counter to what we did when we turned off frontend-js-jquery-web by default (where we want people to get it only if they explicitly turn it on). And speaking of frontend-js-jquery-web, having two places that compete to add a global jQuery instance (ie. this editor package, and that OSGi module), will be yet another point where we'll need to keep versions synchronized, and we might open ourselves up to races depending on load order and internal state that might exist inside the two jQuery modules.

In other words, the question is, is there a way to configure a11ychecker to make use of a specific jQuery instance that we provide in a non-global way? I would be fine with assigning to window if it is done in a suitably "private" manner (eg. window.__LIFERAY_CKEDITOR_INTERNAL_$ or something similarly "private" looking).

About the adapter, I am not sure if it is active by default. git grep adapters/jquery and git grep '\bckeditor\(' don't show any obvious explicit use in liferay-portal. In a running portal instance looking at a CKEditor editors, I see a $ global already (sigh... where is that coming from? I don't think it is AUI, because AUI.$ is unset.), and no $().ckeditor() function, which suggests the adapter is not automatically active. I think it might just be sitting there dormant inside CKEditor and won't have any effect unless loaded/used.

About the adapter, it is optional. It provides deep integration of CKEditor and jQuery that lets you use the native features of jQuery when using CKEditor. It is only needed if you want to be able to use CKEditor as a jQuery component, for example:
$( 'textarea.editor' ).ckeditor();

About the point on using a provided jQuery instance, I will now investigate this and today work on it.

[wincent]

It provides deep integration of CKEditor and jQuery that lets you use the native features of jQuery when using CKEditor. It is only needed if you want to be able to use CKEditor as a jQuery component, for example:
$( 'textarea.editor' ).ckeditor();

Yeah, I know, as you can see from the adapter documentation that I linked.

[wincent]

Just to reflect some chats in Slack and various other places (principal prior discussion was on this PR), here is a bit of a rundown:

• At the moment, we have this PR, which integrates the plugin by prepending jQuery to it; this has the unfortunate side-effect of exposing jQuery on the window, which in turn means that DXP winds up accidentally/unofficially "supporting"/providing jQuery by virtue of that fact that CKEditor is our standard WYSIWYG editor. This approach modifies our liferay-ckeditor package (pros: this keeps modifications local to the package, and leverages the fact that we already have a custom build process in place in this repo).

• We have an alternative approach on @mateomustapic's fork here, which works by pulling down jQuery from the CDN. In addition to the undesirability of relying on a CDN (some DXP customers are sensitive about that; we should provide everything needed directly), it suffers from the same problem, which is that it leaks jQuery into the global environment. This approach does not modify the liferay-ckeditor package; rather, it includes a modified copy of the plugin source in the DXP repo and uses Gradle to stitch it all together (pros: follows an established pattern as we're already doing this quite a bit; cons: the changes made to the upstream source are not clear because it is just a dump of partially minified plugin source, and plugin source will need to be updated manually with each release).

• We need jQuery because a11ychecker depends on it, and a11ychecker depends on it because Quail, the accessibility library, depends on it.

• It is in theory possible for us to hack jQuery to not assign itself to the window (since 2011); specifically, (I think), we would need to make a custom AMD-style build using (this entry point), and then wire that up into our build process. We could then patch the plugin source to not read window.jQuery; as you can see from the source on Mateo's fork; there are a number of explicit and also implicit global reads (eg. naked jQuery) that would need to be patched. Quail's own uses of window.jQuery would also need to be patched. My understanding is that the Quail source is effective bundled into the plugin source, although it is hard to see due to minification. For example these minified lines look like they correspond to this section in the Quail source. In other words, we don't have to audit/transform Quail separately; by auditing/transforming the a11ychecker source we cover Quail too.

• But, even if we can do this (and I believe we probably can), the question is, do we want to? Quail was last committed to in 2016 and is now archived. The deprecation notice says:

  All projects have a useful lifespan. Quail was born during a time when accessibility testing, as a discipline, was maturing. There were numerous teams building solutions in parallel -- Quail was just one of these. In the end, we were all orbiting around the same general approach. Technology may have been a distinguishing factor a couple years ago, but even these advantages have largely dissolved as solutions have evolved and improved over time.

  Given the realities of the market and the limited time that the Quail team can devote to this project, we are initializing deprecation for this project. Folks are welcome to fork it or volunteer to maintain it, but realistically, there are better options out there.

• The project is literally dead, and tying ourself to it (and to jQuery) seems like a bad idea, even if we can do it with a grandiose hack that prevents jQuery from leaking out. Quail recommends axe-core as an alternative, but a11ychecker has not migrated to it yet.

• axe-core has a closed ticket asking for CKEditor integration. Discussion moved to the a11ychecker tracker instead; that ticket has been open since 2017, which recent comment activity on it, which to me indicates that either integrating axe-core is either non-trivial, or nobody cares enough to have done it yet... A similar ticket on Drupal has also been open for years and has been active (comments) without actual progress (code).

• Find an alternative to QuailJS cksource/ckeditor4-plugin-a11ychecker#253 mentions Pa11y as another option, but there is even less visible interest in that.

In short, to summarize our options, I think we have the following choice:

1. Proceed with current plan, and let jQuery leak out. I would be very sad if we chose this.
2. Proceed with a11ychecker/Quail, and hack to prevent jQuery leaking. Given the status of Quail, I would also be a bit sad about this (less sad than "1", though).
3. Do the work that, to date, nobody else has seen fit to prioritize, to integrate axe-core into a11ychecker. This seems like "The Best" option, but we would need to evaluate the cost. My gut instinct is that it is going to be more work than "2" (which itself is more work than "1").

Given that this is a question of prioritization and resource allocation, I think we are going to need PM involvement.

[wincent]

Requesting changes to prevent accidental merge.

Until @jbalsas has a chance to discuss investment with @david-truong, this one is on hold.

[jbalsas]

1. Proceed with current plan, and let jQuery leak out. I would be very sad if we chose this.
2. Proceed with a11ychecker/Quail, and hack to prevent jQuery leaking. Given the status of Quail, I would also be a bit sad about this (less sad than "1", though).
3. Do the work that, to date, nobody else has seen fit to prioritize, to integrate axe-core into a11ychecker. This seems like "The Best" option, but we would need to evaluate the cost. My gut instinct is that it is going to be more work than "2" (which itself is more work than "1").

Or 4... do nothing :)

[wincent]

Or 4... do nothing :)

Always a good option.

[david-truong]

1 seems like a horrible solution since we spent so much time removing jquery from our codebase

[jbalsas]

So, I think we can close this since we won't be going forward at least in this way.

I don't think anyone's excited about adding back jquery and a plugin that depends on a years-long abandoned library, so... let's kill this for now and rethink the approach. We can prioritize once we've discussed it and create a new independent epic for this.

Closing!

[duracell80]

React will fade away one day. jQuery will always be there for you when needed.

[wincent]

jQuery will always be there for you when needed.

Except in DXP, where it is disabled by default since 7.3 (LPS-95726).