Closed albinazs closed 1 year ago
adamchaboryk
commented
1 year ago Hey @albinazs, that's exciting to hear you'd like to integrate Sa11y!
Can you please expand on what you mean by custom UI? Do you mean shadow DOM support? What kind of props to customize the UI?
albinazs
commented
1 year ago Hi @adamchaboryk! Thanks a lot for a quick reply. We're now having an internal discussion, and soon my senior colleague will join this thread
thibaudcolas
commented
1 year ago 👋 Hi! I work with Albina :) Thank you for responding to this so quickly. To expand on Albina’s request, we’re interested in using Sa11y in our CMS if we can integrate it enough that it’s available exactly where authors would expect it to be.
In our case that means two places to start with:
To cover those cases, I believe we’d ideally want to use:
We’d love to have your thoughts on how much this might align with any existing plans you have for the project, if at all, and which of those areas you’d be up for seeing us contribute to. Please let me know if more information would help.
adamchaboryk
commented
1 year ago Hi @thibaudcolas and @albinazs,
Thanks for the explanation, this is helpful. I also want to ensure that Sa11y is the most appropriate choice for Wagtail in your proposed context. I assume this "preview mode" is geared towards content authors? Sa11y is a great choice if you are intending to highlight content related accessibility issues. Although if Wagtail is more developer oriented, you may also want to have a look at axe-core.
Just out of curiosity, how do you envision displaying the results then?
Coincidentally, I recently did some refactoring where I push each individual issue as an object to an array. Thanks to the recent refactoring, I was able to quickly create a prop to run Sa11y in "headless" mode today. Although I'm not quite sure yet how I'd cover your third case.
Here's an example of getting the results array from Sa11y in "headless" mode from my development branch:
Example initialization using headless prop.
Sa11y.Lang.addI18n(Sa11yLangEn.strings);
const sa11y = new Sa11y.Sa11y({
checkRoot: "body",
headless: true
});Then simply refer to the results array to get a list of all issues: sa11y.results. In your case, I'd assume all you would need is Element, Type, and Content from each "issue" object.
Let me know if this is helpful at all - or please let me know if you have any other requirements.
Cheers
albinazs
commented
1 year ago @adamchaboryk thank you a lot for providing a solution so quickly! We will go ahead and use it for our live preview panel, which is indeed geared towards the content authors.
In the meantime, we might need RTL languages support for sa11y UI - do you have such plans, or maybe we can contribute to it? Also, we detected a few potential bugs - do you have any list of issues you're working now on? Or shall we just open a new issue once it's confirmed?
Cheers!
adamchaboryk
commented
1 year ago Thanks Albina. It was quick to implement, but I still have lots of testing to do on my end. My latest commit ensures that toggleable rulesets work in 'headless' mode. I haven't come across any potential bugs yet, so please create new issues for anything you come across.
List of things I'm working on:
The latest dev branch I'm working on has lots of changes. Given the upcoming holidays, this release is still far away. So feel free to explore and let me know what issues you come across, or any other requirements.
Cheers
brianteeman
commented
1 year ago RTL already exists - at least it does in our fork (which we desp[eratlety need to find time to move back to use your updated code)
I just need the translators for arabic to catch up
adamchaboryk
commented
1 year ago Awesome, thanks @brianteeman - glad I tagged you. I'll have a peek at your RTL code.
I was meaning to reach out to you about that. I also got Joomla working on my local - going to try and poke around while I have some time this month.
brianteeman
commented
1 year ago 99% certain that it was just css for rtl and probably logical properties so it was the same css file
thibaudcolas
commented
1 year ago Great to see so much activity here :) It’s great to see this discussion being potentially useful for other CMSes!
Getting back to your questions Adam, we did consider Axe as well. Currently we believe Sa11y is better suited for our needs since its checks are more content-focused, its built-in UI (annotations, panel) looks excellent, and it has added semi-automated QA tools like outline / readability that our authors would really love.
Wagtail’s preview mode is indeed geared towards content authors. For context, here’s what that preview mode would look like if we integrated Sa11y as-is. This is our "page editor" interface in the CMS, editing a blog page titled "Nordic bread culture", with Sa11y displayed in the live preview panel to the right:
We’re interested in "headless" Sa11y so we can:
Aside from the requirements I shared earlier, there are more detailed points I can expand on:
window.postMessage): Sa11y would run the checks inside the page preview’s iframe, and we’d display the results directly in the CMS UI. So we’d likely want them to be JSON-serialisable (store a CSS selector / path rather than the element Node)That’s the ones I’d expect to be relevant to other Sa11y users as well. Not to say all of those need to be changed in Sa11y itself, but if there’s agreement those are desirable I’d certainly prefer we make pull requests to Sa11y rather than work around those things in our implementation.
Finally – one of the critical requirements that has a lot of implications is that we’d like to have all of those checks turned on by default for all Wagtail sites, for all authors. The biggest implication is the need for those on-by-default checks to have a false positive rate as close to zero as possible: we’ll only report issues that are real issues, and that are content issues.
Has there been any benchmarking of Sa11y / Tota11y done so far? I see there are demo pages with errors on Sa11y’s demo / test site and Jooa11y’s site. Ideally we’d want something like GOV.UK’s accessibility tool audit results.
brianteeman
commented
1 year ago wow - you dont ask for much ;)
personally I suggest that you look at doing what we did with Joomla. At the time we found sa11y there was a long list of changes that we would need. So we forked the code and developed the changes and then made those changes available to sa11y. If I am right all the changes we made have now either been directly implemented in sa11y or have inspired the changes.
sa11y is virtually a one person project and unlike wagtail doesnt charge me for developing new features. Perhaps you should raise the funds to do this work (https://wagtail.org/sponsor/) as it is a very significant workload to implement everything you have asked for.
personally I think its the wrong tool for you as aXe is a much better fit.
thibaudcolas
commented
1 year ago To clarify – I’m not asking for anyone to do any of the above. I’ve shared those requirements because @adamchaboryk said he’d be interested in us doing so. Those requirements are for our use case, not things we necessarily expect to see Sa11y itself cater for.
At this stage all we’re interested in is knowing how much those align with Sa11y’s design goals / roadmap. Then yes, if there’s agreement that’s the right way forward, we’d like to make those changes directly in Sa11y. Based on feedback here, we can:
This is what I understood to be possible based on Sa11y’s contributing docs, though please let me know if this doesn’t feel right.
As far as funding – I don’t understand what you mean by charging for developing new features? As far as I’m aware Sa11y isn’t looking for financial sponsorship. We did raise funds for this work, @albinazs who opened this issue is working on this as part of an internship with Wagtail within a program called Outreachy, as a first experience with open source web development. Her work is funded by Torchbox (my employer, original authors of Wagtail) and Outreachy.
Thanks for the feedback re Axe – I’ll share our research that led us to the opposite conclusion if given the chance, would be interesting to get more perspective on this.
adamchaboryk
commented
1 year ago Indeed, it's great to see so much activity here. 😁
Just for some historical context, Joomla is the first CMS to implement Sa11y as a core feature. Joomla implemented it in February of this year. @brianteeman and company were instrumental in helping me modernize Sa11y while ensuring it remains platform/CMS agnostic. As Brian said, Sa11y is virtually a one person project without any funding. It's something I maintain on the side (whenever I have time throughout the year and usually during working hours) - hence why I do not ask for financial support. This project was started at Toronto Metropolitan University (my current employer).
Sa11y's main goal is to be a platform agnostic content accessibility checker. So any changes to Sa11y would need to ensure it remains as such - so it plays nicely with Joomla and other CMSs. Otherwise, feature requests, changes and PRs are welcomed with discussion!
I'll take a look at some of these requirements: some of them may need another discussion, and with some I will most definitely need help with.
Headless
My headless prop is basically complete, although needs more testing. I'll probably ask for Wagtail's help on this given headless is your expertise. I'll investigate into pushing a CSS selector in each issue object in addition to the element node.
TBC – may well be supported already: We’d need a way to re-run Sa11y’s checks every time the preview’s contents gets updated
I think this is something you'd have to implement on Wagtail's side. Otherwise, what were you thinking? MutationObserver? Not sure how you would do this performantly.
TBC – may well be the case already: Make sure Sa11y’s UI is 100% WCAG 2.2 AA conformant
Not 100% - there are some exceptions. For example, by default tooltips are appended to the end of the document's body to avoid conflicting CSS that may prevent tooltips from getting hidden. This poses a barrier for interactive content in tooltips. However, in my latest branch I've included a prop that gives developers ability to decide where tooltips can go. Looking into a custom focus solution is also on my to-do list.
Nice-to-have – and apparently already on your radar? Wrap Sa11y’s code in Web Components (custom elements + shadow DOM) so we 100% guarantee it won’t affect or be affected by the site’s page it’s displayed on. Nice-to-have – theme Sa11y’s appearance (colors) so it reuses our admin interface’s theme, which is configurable by developers (just a long list of CSS variables)
Despite not using shadow DOM, my CSS reset works pretty great. It's rare I see conflicting CSS issues. Although of course, this is a nice to have for me as well. This is a bit more complicated to do given the current architecture of Sa11y. I may need help with this too.
Nice-to-have – theme Sa11y’s appearance (colors) so it reuses our admin interface’s theme, which is configurable by developers (just a long list of CSS variables)
If you check out the .scss file - all the theme variables are at the top (first 100 lines of the file). Does this suffice?
Finally – one of the critical requirements that has a lot of implications is that we’d like to have all of those checks turned on by default for all Wagtail sites, for all authors. The biggest implication is the need for those on-by-default checks to have a false positive rate as close to zero as possible: we’ll only report issues that are real issues, and that are content issues.
I can envision a prop for this to make all rulesets on by default if using Sa11y's UI. However, using my headless prop in my dev branch - it checks everything.
Over time, I've done a pretty good job of minimizing false positives while keeping Sa11y lean. But getting a false positive rate of 0% is extremely difficult. For example, ARIA computation and accessible name calculation is very complex. I've managed to get a pretty accurate accessible name computation with about 70 or so lines of code. But there are some scripts out there that are thousands of lines of code that only focus on accurate ARIA computation. That said, Sa11y is primarily for content issues only - it doesn't need to accurately calculate complex accessible name calculations! I've also included props for all QA rulesets to provide administrators/developers more flexibility in only showing "real" issues too.
I initially suggested Axe because it's more API oriented out of the box for your headless use case, but also because it is one of the most false-positive free engines.
Has there been any benchmarking of Sa11y / Tota11y done so far? I see there are demo pages with errors on Sa11y’s demo / test site and Jooa11y’s site. Ideally we’d want something like GOV.UK’s accessibility tool audit results.
There is none. Nor do I have any plans to do any myself.
I'm happy to keep this conversation going, and I'm open to receiving contributions/PRs as long as it aligns with Sa11y's goal of being a lean, platform agnostic content accessibility checker. Ideally, we'll be able to collectively support both Joomla and Wagtail... (and my WordPress plugin)... and any other CMSs looking to step up their accessibility game. :-)
thibaudcolas
commented
1 year ago Yay to keeping the conversation going!
I'll investigate into pushing a CSS selector in each issue object in addition to the element node.
I‘d advise against this if keeping Sa11y lean is a clear goal. I’m sure there is a library we could use for this on our side – and if more people needed this in the future, it’d be straightforward to add at that point with the trade-off weighed in.
Re re-running Sa11y – in headless mode, I think this would just be a matter of documenting a method of the Sa11y instance that can be called as needed to re-compute the results. Then we can elect to trigger this with Mutation Observer or something else.
Re web components, I think that’s something we’d happily do. Use a custom element for the trigger, for the panel, and for the annotations. Each with their shadow DOM with as-is code.
Even if the current CSS reset works as-is – having simpler CSS will be easier to maintain and will make the styles simpler to customise.
If you check out the .scss file - all the theme variables are at the top (first 100 lines of the file). Does this suffice?
Yes! Surprised I missed that.
I've also included props for all QA rulesets to provide administrators/developers more flexibility in only showing "real" issues too.
That’s the most important to me - we can keep to the subset of Sa11y checks which we think works well enough for us.
And for the benchmarking, that’s something we’ll happily do.
I have three more questions for us to assess the suitability of Sa11y, that came up as part of our research in UI customisations.
We’ll likely need to re-implement some parts of Sa11y so we can make use of it. This feels reasonable to me but we’re running into the issue that Sa11y v2.1.0 and up is now GPL-licensed. This creates a fair bit of headache for us. With an optimistic / potentially naive outlook, only our overrides to Sa11y would be considered derivative works, and as such would have to be implemented in a separate project that we’d need to have GPL-licensed. With a more pessimistic / realistic outlook, even our basic integration of Sa11y regardless of any overrides would need to be GPL-licensed – and as such could only be a fully separate, opt-in plugin for our CMS.
Was that your intention with this re-licensing? Did you or would you consider a "weak" copyleft license such as LGPL or MPL? From our perspective those are much easier to work with and we’d be confident we could use Sa11y without the drawbacks of having a separate project for our integration. I believe LGPL would still require us to have a separate project, while with MPL we’d be able to only change the license of the specific files in which we have derived work based on Sa11y, while keeping those files within the
I couldn’t find a reference to whether Sa11y attempts to support specific assistive technologies or not / what it’s been tested with so far. We did brief testing with VoiceOver, and Windows High Contrast Mode (WHCM). There doesn’t seem to be any support for WHCM currently – is this something we could contribute?
Last but not least – apologies if this is documented and I missed it, what’s the browser support target for Sa11y? Aside from general "evergreen" support, the main source of concern for us is support for mobile devices / touch screens, and not-so-evergreen Safari release (Safari 14 is our current cutoff).
Sorry to be so needy with information at this point, I believe the above three should be the last things we have to consider though.
brianteeman
commented
1 year ago Why is GPL licensing a problem? ianal - Integrating Sa11y into your BSD project would not impact on the existing licensing of wagtail
thibaudcolas
commented
1 year ago The gist of the issue is the GPL being a strong copyleft license. There are two factors,
First, very clear-cut: Any copy we’d make of some of Sa11y’s code would be considered a derivative work and would have to be GPL-licensed. For example if we copied the computeAriaLabel implementation to change the logic slightly – that’d have to be GPL-licensed. This means we’d have to do this in a separate repository if we don’t want to change the license of the CMS.
Second, the more crucial point that is not resolved is whether using and bundling a GPL library constitutes a derivative work. The Wikipedia article is pretty clear so I’ll just copy it:
This key dispute is whether non-GPL software can legally statically link or dynamically link to GPL libraries. Different opinions exist on this issue. The GPL is clear in requiring that all derivative works of code under the GPL must themselves be under the GPL. Ambiguity arises with regards to using GPL libraries, and bundling GPL software into a larger package (perhaps mixed into a binary via static linking). This is ultimately a question not of the GPL per se, but of how copyright law defines derivative works. The following points of view exist: […]
The LGPL (and MPL) are both resolve this dispute / ambiguity. Wikipedia’s LGL differences from the GPL content is a good recap:
A program that contains no derivative of any portion of the Library, but is designed to work with the Library by being compiled or linked with it, is called a "work that uses the Library". Such a work, in isolation, is not a derivative work of the Library, and therefore falls outside the scope of this License.
brianteeman
commented
1 year ago IANAL but the legal advice Joomla received (from SFLC lawyers not wikipedia) is that in this scenario sa11y is a script not a library in the definition used by the fsf. As such bundling sa11y with wagtail would not change wagtail to be gpl. Of course any changes to the sa11y script would have to be gpl
thibaudcolas
commented
1 year ago 👍 that’s an interesting distinction I’ve not come across before. Is that case what’s described in the official GPL FAQ under #WMS? If you know of any further resource covering this please do share.
brianteeman
commented
1 year ago I think the graphic from the article is useful
That's how my previous comment above has been explained to me although I hadn't seen that graphic before. However it is only fair to point out that the desciption on that article is referring to the content copyright even if the graphic clearly explains the exact scenario we are referring to here.
The FSF have historically been very obtuse and silent on the licence requirements of css and js and always said "its user content or client side so not code" so it doesnt apply. not very helpful
The following post on stack exchange is very informative https://opensource.stackexchange.com/questions/4360/what-are-the-implications-of-licensing-a-javascript-library-under-gpl?rq=1 As are these pages referenced in stack exchange https://hroy.eu/posts/gpl-js-bs/ https://greendrake.info/publications/js-gpl
brianteeman
commented
1 year ago Forgot to add. By including sa11y, jquery, tailwind etc with the download of wagtail and/or in the same repo what you are doing is aggregating the programs and that is perfectly acceptable.
I try to think of aggregation as the same as the cd for Now Thats what I call music vol 22. All the songs are still the copyright the original authors
https://www.gnu.org/licenses/gpl-faq.en.html#MereAggregation
adamchaboryk
commented
1 year ago I re-released Sa11y under GPL in the spirit of open source, with hopes that any improvements would make it back upstream to my repo. I also want to ensure that Sa11y always remains free, as tools like this ultimately help authors create accessible content.
Secondly, many popular CMSs (namely Joomla, WordPress, and Drupal) also require plugins to be GPL licensed too. This was a requirement for when I submitted my WordPress plugin. For these reasons, I do not want to change the licensing at this time. My intention is not to be restrictive - I would love to have other open source CMSs incorporate Sa11y.
Also NAL, I think I can hypothetically license Sa11y?
I‘d advise against this if keeping Sa11y lean is a clear goal. I’m sure there is a library we could use for this on our side – and if more people needed this in the future, it’d be straightforward to add at that point with the trade-off weighed in.
For fun, I implemented a lightweight function (dozen lines of code) that generates a CSS selector path in my dev branch. No idea if this is accurate enough or at all, given that there are libraries dedicated to this very task. But like you said, should be very straightforward if you wanted to leverage another library for this. Feel free to check it out, otherwise I may scrap it.
I couldn’t find a reference to whether Sa11y attempts to support specific assistive technologies or not / what it’s been tested with so far. We did brief testing with VoiceOver, and Windows High Contrast Mode (WHCM). There doesn’t seem to be any support for WHCM currently – is this something we could contribute?
Last but not least – apologies if this is documented and I missed it, what’s the browser support target for Sa11y? Aside from general "evergreen" support, the main source of concern for us is support for mobile devices / touch screens, and not-so-evergreen Safari release (Safari 14 is our current cutoff).
No need to apologize. If there's a lack of documentation, it's either because: no one has asked me for it, or I simply haven't had time to get around to creating it (one person side project).
Regarding accessibility, I try to practice what I preach as the creator of an accessibility checker. :-) Sa11y's main panel uses native semantics, so it should work well with all major assistive technology. As a Mac user, I primarily do my testing with VoiceOver. I have some CSS for WHCM, but this isn't something that's easy for me to develop without a Windows PC. So yes, I'd be totally grateful for WHCM support!
I think I'm done. Needs peer review & testing.
Excited to see so much discussion on this thread!! I think i'll create another thread or enable GitHub's discussion/projects feature to organize some of the items discussed. Or something to simply highlight items that are on my radar, things i'm currently working on, and things I need help with.
Thank you all for your comments and thoughts. Have a great weekend!
brianteeman
commented
1 year ago ping me when you want the rtl testing. I do a lot of work in RTL and there are lot of weird things that can happen but I am aware of them so will spot them quickly if present.
brianteeman
commented
1 year ago Re testing on windows I am adding you to my browserstack account - they gave me it for jooa11y so its fair use
adamchaboryk
commented
1 year ago Thanks Brian, I'll give you a ping when I'm ready.
Thanks for the offer - I actually have a free BrowserStack account (for Sa11y) - I use it to test after significant changes.
brianteeman
commented
1 year ago Doh - it was you that told me about browserstack
adamchaboryk
commented
1 year ago Heck, i forgot I told you. 😅 I'm ready for the hols.
adamchaboryk
commented
1 year ago Created another thread for 2023 roadmap. Although happy to continue some of the discussion here.
thibaudcolas
commented
1 year ago Hi folks, following up on the licensing discussion – we’ve decided it’s enough of a concern for us that we won’t integrate Sa11y directly in our CMS. We’ll use Axe with a simpler bespoke interface for the "on by default" checks we have in the core project, and create a Sa11y integration as a separate opt-in GPL-licensed plugin.
The research we’ve done confirms our integration with Sa11y would be considered a derivative work. To keep it short:
In addition, as-is, we believe we would have to override significant parts of Sa11y to make use of it with a custom interface, which would require us to make this copy + overrides as a fork / derivative package so those customisations can be GPL-licensed. Not a blocker but certainly a hurdle we would prefer to avoid.
My intention is not to be restrictive - I would love to have other open source CMSs incorporate Sa11y.
IANAL but as far as I understand based on the research we did there’s a contradiction here. If you want to have other open source CMSes integrate Sa11y, using a permissive, not-copyleft license is the simplest way (MIT, BSD, etc). Sa11y itself being MIT-licensed is a non-issue as far as integration in GPL projects – because as you mention Sa11y itself is CMS-agnostic. Any code that is specific to e.g. Joomla or Wordpress for integration purposes would have to be GPL-licensed. Sa11y itself isn’t responsible for integrating with either.
If the goal is to only restrict usage to open source CMSes – then definitely it’s the right call to use the GPL, but keep in mind it does restrict any integration to also be GPL-licensed.
Back to our usage of Sa11y, now as a separate plugin:
headless support, at least not for the time being.brianteeman
commented
1 year ago That is of course your interpretation and you are of course perfectly allowed to adopt that. I personally disagree
adamchaboryk
commented
1 year ago Thanks @thibaudcolas, I understand your concern. Your proposed alternative is also a great solution, especially if Wagtail is more developer-oriented.
If there's a contradiction on my end, it's unintentional because IANAL. 😅 I'll give (re)licensing some more thought. I think BSD may be a suitable license, despite the risk of contributions not going upstream. Given the significant contributions by @brianteeman and the Joomla team, I would also want their input and approval if we were to change the license. Something to think about over the holiday break.
I'm looking forward to future collaborations!!
adamchaboryk
commented
1 year ago Closing this thread for now, given that most items have been implemented. Thank you all once again for your feedback and support!
Hi there! We at Wagtail, open source Python-based CMS, want to assist authors in making their Wagtail-based sites as accessible as possible. We are considering to integrate sa11y’s test suite as a part of our userbar widget on the page preview.
Do you plan to add support for using sa11y’s functionality with custom UI, or customising sa11y’s UI with props so that it works out of the box? Looking forward to hearing your thoughts.