Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add shadow root handling #476

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

deleonio
Copy link

@deleonio deleonio commented Apr 9, 2022

Hello,

in the last year the custom elements come up and more and more websites and web apps contains web components with shadow doms.

With the small code extension, shadow roots can now also be handled.

I am happy if I can contribute to more accessibility.

@deleonio deleonio force-pushed the feature/add-shadow-root-support-for-finding-landmarks branch from 8dca5de to d750943 Compare April 9, 2022 03:03
@deleonio
Copy link
Author

deleonio commented Apr 16, 2022

Hi @matatk, could you merge my PR, please?

@matatk
Copy link
Owner

matatk commented Apr 16, 2022

Hi @deleonio, thanks for your PR. Like you, I have seen more custom elements recently.

The ARIA in HTML document mentions custom elements in the context of determining which ARIA role they would naturally expose, and references the ElementInternals interface in the HTML spec. This interface allows the custom element's creator to expose a single role-however, as I understand it, that role would be exposed without the need to traverse into the Shadow DOM.

From what I'm reading here, I get the impression that custom elements are expected to be small, and have maybe one outward-facing role. I don't see anything that forbids them exposing a landmark role, but my understanding is that whatever is in the Shadow DOM would be kept separate from the host page, and wouldn't need to be traversed in order for that role to be exposed.

Are there any pages you're aware of that contain landmarks inside custom elements? I'd like to see how assistive technologies such as screen readers handle them.

I am wondering, if a custom element exposes a role that happens to be a landmark role, e.g. "search", if the Landmarks extension would pick it up (and the same for other assistive technologies).

If you know of any examples out in the wild that I could check out with assistive technologies, please let me know.

@deleonio
Copy link
Author

Thank you, I've learned something new from you.

But the point is that we create accessible high-level components strictly semantically according to W3C and WCAG. Under certain circumstances, these can also contain several roles.

Is it possible to get your mail address for a private demo link?!

github@martinoppitz.com / subject: "landmarks chrome"

Thank you and happy eastern!

@deleonio
Copy link
Author

In other words - the Webcomponents web standard allows it to be used in a wide variety of scenarios - including those described above.

It would be great if you merged the small improvement.

@matatk
Copy link
Owner

matatk commented May 12, 2022

Hi @deleonio. Sorry for my slow reply; I didn't get the notification of your reply. However I did email on the 4th—hope it got through. If not, we could try Twitter perhaps (I have the same handle there). I am interested to see the components you are working on.

@deleonio
Copy link
Author

Hi, checked. Nop. I will send you a Mail for testing.

@matatk
Copy link
Owner

matatk commented May 25, 2022

Hi again @deleonio. I've tried emailing again; hope it works this time. I will be offline for some of the coming week, but will be able to look into this again late next week. I am looking forward to trying out your custom elements. I'm also doing some research into custom elements and accessibility here too.

Also, as it happens, just before your PR came in, I was working on improving the performance testing for the Landmarks extension, so it should soon be much easier and more reliable to find out how any changes affect performance.

@matatk
Copy link
Owner

matatk commented Jul 9, 2022

@deleonio: sorry for being slow on this; my free cycles have been pretty limited, and are still, but I haven't forgotten about this. I wasn't able to make email contact, but I am still intending to investigate this more thoroughly, and will let you know what I find as soon as I'm able!

@matatk
Copy link
Owner

matatk commented Aug 7, 2022

@deleonio: I realised that I could test the performance of your PR before I merge the scanner improvements that I started back at the beginning of April (more details on those changes below, in case you're interested).

I ran the tests, and found that the added checks for custom elements cause only a 3% slowdown against the main branch, which is very good. However, I still haven't found any good publicly-available samples of pages with custom elements suitable to use as test cases. I'm also still a little concerned about even a 3% slowdown, if custom elements are in practice very rarely used—but the work I've been doing on improving the performance of the scanner will hopefully more than balance that out.

tl;dr: Continued thanks for your contribution (and sorry it's taking so long to merge it); I do plan to accept it as soon as I can. I am still looking for suitable test cases in the wild before I will feel comfortable in doing so, though. If you find any publicly-available examples, please let me know. I will keep looking.


Incidentally, the scanner changes I've been working on since the start of April are in support of #389. Here are some of the things I have been working on:

  • combine-scanners-split-profile simplifies the starting point for the following changes by re-combining the "standard" and "developer" mode scanners. In the interests of performance, I might still re-split them again later, but we'll see.
  • landmarksfinder-internal-tree-structure changes the LandmarksFinder to work with a tree structure, as opposed to a flat list of landmarks, so that in future only sub-trees of the DOM need to be scanned when there are mutations.
  • ui-use-tree changes the internal logic of the GUI landmarks tree-like display to work with the internal landmarks tree structure, rather than a flat list.
  • test-mutation-debugging Is still in progress, and currently consists of me trying to improve the test suite so that I can see how performance is affected when encountering mutations of specific types. I haven't yet made the LandmarksFinder recognise mutations.

I haven't merged any of these yet, because I wasn't sure if I was going in the right direction, but hope to be able to merge them soon. My hope is doing so will make the extension much more efficient.

Either way, if we can ascertain that there are a reasonable number of custom elements out there that would benefit from your PR, I'd be happy to merge it in the mean time.

@matatk
Copy link
Owner

matatk commented Aug 30, 2022

@deleonio: I'm still blocked on needing public examples of pages with custom elements containing landmark regions, to test with assistive technologies (and I'm still looking).

If I get into a position where I can merge this, do you agree to assign the copyright for your changes to me? I don't have "official" contributing guidelines yet, but I think if you were to assign copyright to me, that would be sufficient from a legal/licensing perspective. I would also mention you in the acknowledgements, of course!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants