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

Landmarks in iframe not appearing in list #370

Open
moekraft opened this issue May 28, 2020 · 17 comments
Open

Landmarks in iframe not appearing in list #370

moekraft opened this issue May 28, 2020 · 17 comments

Comments

@moekraft
Copy link

Hi @matatk,
Happy to see you keep this plugin alive!
I have an application that runs inside of an iframe. When I test the standalone iframe in my dev environment, all of the landmarks in the iframe appear in the Landmarks list and are highlighted on the page.
image

If access the page in our production environment, I only see the landmarks outside of the iframe down to the main landmark which is defined by the containing application.
image

I tested the production environment with JAWS and see all of the Landmarks including those from the parent application and those inside of the iframe. I am wondering if this is a limitation to the Landmarks plugin or a known issue.

@moekraft
Copy link
Author

Hi Matt, I think I found the issue. In JAWS, it appears we have defined two Main regions.
LandmarkRegions

@carmacleod
Copy link

@matatk
After you add linting support, maybe it would be ok for Landmarks to "support" multiple mains even though it is invalid?
This would be in line with what browsers do, "supporting" all kinds of invalid code the best way they can.

@matatk
Copy link
Owner

matatk commented Jun 11, 2020

Thanks @moekraft. I've not considered this situation before; thanks for raising it :-).

tl;dr

I can't address this immediately, but there are some avenues I am exploring. I hope to resolve it soon, but it won't be in the next release.

There's a section about the multiple main regions below, too.

Background

The duplicate main region shouldn't be causing the problem—it's that Landmarks doesn't look inside the <iframe> in the required way (it goes through the frame's .children but that doesn't include the contained, but separate, document).

I agree that it's a bug that Landmarks doesn't traverse into the <iframe> content. The ARIA spec doesn't mention landmarks and <iframe>s explicitly, but other assistive technologies do surface them, as you've verified, and that seems to match what a user would expect.

Unfortunately, it's not easy to make this work in the context of a browser extension. I've done some quick checks, and the security model doesn't appear to provide access to the content of an <iframe> even when it's on the same domain as the parent document (this isn't necessarily a bad thing—it just means I have to work a bit on this :-)). You can have the extension's code injected into the frames, but the code would require re-writing to fit that piece-wise way of working. I've tried @xi's very cool a11y-outline extension, and it behaves the same way.

For a while now, I've had the 'blue sky' thought that in the very long term, re-writing the scanning to be more tree-based and allow for different sub-parts of the document to be re-scanned when there are mutations could help with efficiency, and it could help here too. Though I have concentrated on making the existing approach as efficient as possible, and I think it's working well, and another even more extensive re-write is not something I can do right now.

(Side note: I've been looking into functional programming properly at long last recently and I wonder, when the time does come, if using a language like Elm might help here.)

However, there are steps in between where we are now and a re-architecting that I can explore; I'll see if I can at least make the code 'frame-aware' after the next release. (I was planning to make a new release quite soon, and this would be something I'd like to run in testing for a while before being sure about it.)

Duplicate main regions

Thanks for your suggestion too @carmacleod. I agree that the approach you propose is in-keeping with the sprit of the web. That's actually how Landmarks works to a large degree already.

Currently Landmarks only keeps track of the first main region it finds on the page with respect to supporting the 'skip to main' keyboard shortcut. However, it does recognise any other main regions and puts them in the pop-up/sidebar.

Right now it will only ever jump to the first main though. Do you think the 'skip to main' shortcut should cycle between all discovered main regions, rather than only going to the first?

JAWS has a key (Q) that moves to the main region, and if there are more than one, it cycles between them. Other screen readers don't seem to have a key for going directly to the main region specifically; rather they have ones for cycling through them.

@xi
Copy link

xi commented Jun 11, 2020

From what I know about iframes there are some cases where it is impossible to to get their content from JavaScript (e.g. if the iframe comes from a different origin). I am not sure if it is worth the effort to add support for the few cases where it is possible.

My approach would be to live with this limitation and document it.

@carmacleod
Copy link

Do you think the 'skip to main' shortcut should cycle between all discovered main regions, rather than only going to the first?

Good question. If JAWS cycles, maybe Landmarks should? Or, only go to first main with the "skip to main" command as long as the "next/previous landmark" command cycles through multiple mains.

@matatk
Copy link
Owner

matatk commented Jun 14, 2020

Thanks for your input @xi. It does seem like this would be a lot of work, even if there's a guaranteed way to get the frames' content. I’m not sure how many pages in the wild would both use <iframe>s and have meaningful interaction within the frames.

The browser can inject the content script into frames—in my local tests this has worked well, but I'm not sure how it handles different origins—but the code would require re-working to fit into that environment. I found the webNavigation.getAllFrames() function (via StackOverflow), which provides the information needed to piece together the landmarks tree from different frames, but that would be a big development effort.

However, I might be able to do pretty quickly is at least include the landmarks from other frames in the GUIs, separately from the landmarks for the main page—then at least they’d be in there. If I do that, and use it, maybe we will get a sense of how often this comes up.

I was going to ask if you think we should have a test case for this in the test suite, but I think it's best that we consider it out of scope there. (I did some local trials (with things like having the <iframe> content in a data: URL) and it didn't work well, and re-architecting the tests to involve more than one file per fixture would add a lot of complexity.)

@carmacleod
Copy link

However, I might be able to do pretty quickly is at least include the landmarks from other frames in the GUIs, separately from the landmarks for the main page—then at least they’d be in there. If I do that, and use it, maybe we will get a sense of how often this comes up.

Sounds like a good first step to me. :)

@moekraft
Copy link
Author

@matatk Thanks Matt!

@moekraft
Copy link
Author

@carmacleod Thanks Carolyn!

@moekraft
Copy link
Author

And discovered that multiple mains are allowed as long as they have unique ids. : ) Glad that you are looking to support landmarks inside the iframe.

@matatk
Copy link
Owner

matatk commented Jul 26, 2020

Hi @moekraft. I have recently released version 2.8.0 of the extension; it does not feature any changes relating to iframes. However I've also just revived the unlisted test variant of the extension and sent a new version for review by the Chrome team. It's no longer as easy to upload a test version for Firefox, but I'm looking into how that might work, as when there are lots of landmarks on the page, it's nice being able to use the sidebar.

Anyway, the upcoming test version...

  • Detects landmarks in iframes and shows them in the pop-up.
  • Lists details on all iframes found (whether they have landmarks or not) in the console for the page.
  • Doesn't work properly when trying to navigate to any landmarks (it tries to navigate in all frames at the same time at the moment, but I wanted to get it uploaded so as to get things going).
  • Isn't able to detect if an iframe is visible on the page, and hence whether it's worth scanning it or not. Making this work would be non-trivial (at least) and is also my main concern, regarding including this feature in the extension. However, I currently don't know how many pages actually have landmarks with iframes in them, so I don't know how much we are missing to not have that feature.

Whilst we await the review, I wanted to ask: will a test version in Chrome work for you, or do you need a version for Firefox, Edge or Opera specifically?

@matatk
Copy link
Owner

matatk commented Aug 3, 2020

The test version of the extension is now available for Chrome—please try it out when you're able and let me know what you think. Now it's up there I should be able to make updates quicker. (One thing that just occurred to me is having a different icon for the test version might be helpful :-).)

@moekraft
Copy link
Author

moekraft commented Sep 9, 2020

@matak Will check out the test extension. Thanks much! I finally got an update through in our iframe with an aria-label to distinguish it from the containing infrastructure. Will test with your test version in Chrome.

@gaurav5430
Copy link

is this still being explored ?

@matatk
Copy link
Owner

matatk commented Aug 30, 2022

Hi @gaurav5430. I haven't (yet) integrated this code into the extension, because the performance overhead would be great, and I haven't encountered any other situations in which scanning <iframe>s would be needed. I am sure such situations exist, but given they seem to be rare, and the efficiency burden would be significant, I haven't taken this further yet.

Also of note: I haven't come across any public pages that have a landmark structure across multiple <iframe>s, which means I don't have any ability to see how current assistive technologies (like screen readers) react to them. Without being able to test that, I wouldn't be able to add such a feature.

I have been working on some potentially significant performance improvements over the past several months (details in a comment on PR #476), and am close to being able to test them. If those performance improvements are very significant, it might be feasible to at least add <iframe> scanning as an option—but I would be concerned about memory usage, which is very hard to debug with the tools that are available at the moment.

@gaurav5430
Copy link

Sure, that's alright and makes sense

in my usage, i was trying to test this inside storybook, which by default renders the UI in an iframe.

i guess i would need to find a storybook specific addon for this

Thanks a lot.

@matatk
Copy link
Owner

matatk commented Aug 30, 2022

That's interesting, @gaurav5430. I too have used StoryBook. I suspect with StoryBook there would not be many <iframe>s on the page (possibly only one), and thus it would not be too big a drain on performance. But the situation that concerns me is a page on the open web, which may contain many <iframe>s used for adverts, for example. Whilst scanning each frame would likely be trivial, I am not sure about the memory requirements.

Another aspect of this is that it's really non-trivial for the code to work out where in the document each <iframe> is (partly for understandable security reasons). This means that landmarks in the frames wouldn't appear in the right place in the tree (at least not without a huge amount of development effort—beyond my available cycles). However, in my test version, I just listed the results for each frame separately, and that is something I could do relatively easily.

Something else that just occurred to me: maybe the extension could simply ignore <iframe>s that don't contain any landmarks on page load. That would be a nice feature, but the memory footprint would still be a concern.

Current priorities for features are: moving to a tree-based structure to improve performance; moving to Google's Manifest V3 API (or the extension will stop working); and looking again at adding headings. I will keep thinking about this, though.

One suggestion for you: you might be able to use the browser's "open frame in new tab" feature to get around this problem for StoryBook—if it works, the frame you're interested in would be at the top level of the page, and Landmarks would be able to scan it.

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

No branches or pull requests

5 participants