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

fix(webdriverjs): Recursively find <frame> #209

Merged
merged 7 commits into from
Apr 12, 2021

Conversation

yokra9
Copy link
Contributor

@yokra9 yokra9 commented Mar 14, 2021

Fixed to recursively find <frame> and inject a script into them, not only <iframe>.

<frame> is deprecated in HTML5, but sometimes use in bad examples when teach modern & accessible coding.
ex: https://barrierfree.nict.go.jp/accessibility/whatsacs/sample/level0/index.html

Thanks!

Reviewer checks

Required fields, to be filled out by PR reviewer(s)

  • Code is reviewed for security

@michael-siek michael-siek requested a review from a team March 19, 2021 14:03
@michael-siek
Copy link
Member

Hi, @yokra9 do you mind providing tests for this fix.

@yokra9
Copy link
Contributor Author

yokra9 commented Apr 6, 2021

@michael-siek thanks for comment! added 2 commits for providing tests for this fix.

  • added unit test for this fix.
    • revived test fixture, inner-configure-frame.html, which might have removed unintentionally.
  • removed unused test fixtures.

@WilcoFiers
Copy link
Contributor

@yokra9 I just wanted to let you know we're still looking to see if we want this change added. As you pointed out, frames were deprecated years ago. Axe has never supported them. We're currently working to improve consistency between the various axe packages we maintain. That includes whether or not they support frames. If we add frame support to webdriverjs, we need to do it to webdriverio, puppeteer, cypress, Java/Ruby/C# selenium, Ruby Watir, etc. That is a fairly substantial amount of work.

I very much do appreciate you setting up this PR. And I apologies for not noticing this before you made additional commits to it. I'm going to bring this up internally and will get back to you soon.

@michael-siek michael-siek changed the title Fix to recursively find <frame> fix(webdriverjs): Recursively find <frame> Apr 12, 2021
@michael-siek
Copy link
Member

Thank you very much for your contribution @yokra9.

@michael-siek michael-siek merged commit 0d20e1f into dequelabs:develop Apr 12, 2021
@yokra9 yokra9 deleted the fix-recursively-find-frame branch April 12, 2021 13:28
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.

4 participants