Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

Add warning that AX_FOCUS_02 is not available from axs.Audit.run() #85

Closed
coderjz opened this issue Mar 29, 2014 · 8 comments
Closed

Add warning that AX_FOCUS_02 is not available from axs.Audit.run() #85

coderjz opened this issue Mar 29, 2014 · 8 comments

Comments

@coderjz
Copy link
Contributor

coderjz commented Mar 29, 2014

I compared the Chrome plugin with my results from running axs.Audit.run() and kept noticing that the warning AX_FOCUS_02 was being included in the plugin but not from axs.Audit.run().

After digging in the source I realized it is because this rule requires the "console API" as it uses getEventListeners(), which is not available in the DOM (i.e. outside of Firebug, Chrome DevTools, etc.). http://stackoverflow.com/questions/9046741/get-event-listeners-attached-to-node-using-addeventlistener

Would it make sense to add a warning about this in the documentation under "The axs.Audit.run() method"? Are there others who might get tripped up by this?

@alice
Copy link
Contributor

alice commented Mar 29, 2014

Good point: it might make sense log a warning during axs.Audit.run(), although I worry that it might be confusing/noisy.

Another option might be to make it a compile flag in grunt; it would probably be necessary to split the audit rules up into two directories to do this. That would at least make it clear which audit rules will be included and which won't.

What do you think?

@coderjz
Copy link
Contributor Author

coderjz commented Mar 29, 2014

I definitely agree that logging another warning in axs.Audit.run() may be noisy, especially because it makes a lot of sense once you know why this is the case.

The grunt idea is great, but it won't let someone know that the axs.Audit.run() (or the Command Line Runner) does not include all possible tests. Unless they dive into the code or compare with the Chrome plugin.

Not sure how common my use case is -- building a test framework in PhantomJS and want to include a flag to log accessibility warnings/errors for our dev team to review.

I believe the code below can at least list the rules that aren't supported by axs.Audit.run()

Object.keys(axs.AuditRules.rules).forEach(function(k) { var r = axs.AuditRules.getRule(k); if(r.requiresConsoleAPI) { console.log("Rule " + r.code + " is not available in automated testing."); } } );

@alice
Copy link
Contributor

alice commented Mar 29, 2014

When would the information about what rules are/aren't included be useful?

That code snippet would indeed find rules which aren't included; how/when would you use that?

@coderjz
Copy link
Contributor Author

coderjz commented Mar 29, 2014

I've been struggling with this, I see two different approaches:

  1. Put a log warning first time axs.Audit.run() is run on the page (noisy, but effective).
  2. Update the docs (not noisy, good for people who read the docs, but not maintainers/testers).
    Also does this impact other tools for example the Capybara plugin?

I'm new to this plugin so I don't know if this is something obvious I should have figured out, or how much this would actually affect other people.

For my use case I plan on just putting a comment in my PhantomJS code.
/* Does not support all test cases that are in Chrome plugin. Run code to see which cases are not included */

@alice
Copy link
Contributor

alice commented Mar 29, 2014

I think it's hard to know when you're running an audit for the first time, so it would probably end up running every time you ran the audit, which wouldn't be good. Throwing some things out there:

  • Audit configuration option to suppress warning; would warn if not present, but allow developer to suppress after the first time if desired
  • Have two code paths in AX_FOCUS_02, one which uses the console API and one which doesn't, and remove the requiresConsoleAPI check altogether. One idea for this would be to check for an onclick (obviously this will miss the vast majority of cases) and also check for a cursor:pointer style.

@coderjz
Copy link
Contributor Author

coderjz commented Mar 29, 2014

I really prefer the first option. Actually what I'm thinking is

  • By default only one warning written per page load, when axs.Audit.run() is called.
  • Do a console.warn() call IF at least one rule requires console API AND we configuration.withConsoleApi is set to false.
  • Lists the rules that are not supported, using code provided above.
  • AuditConfiguration.warnUnsupportedRules = false will suppress the console.warning call

My reason for going against having separate code branches and stuff is that my intent to use the CLI is to automate the testing that a person would have to use the Chrome plugin for. I expected the CLI to support the exact same thing as the plugin and was quite surprised when it didn't. Doing a fix with different behaviour would make it even harder to understand why the logic is different.

Suggested message: "Some rules cannot be checked using the axs.Audit.run() method call. Use the Chrome plugin to check these rules: [LIST THE RULES]

If there is a way to get it to work without the Chrome plugin, I wasn't able to find it -- I tried in the Developer Console to call axs.Audit.run() allowing the console API, and I got a ReferenceError.

Should I edit Audit.js and send you a pull request?

@alice
Copy link
Contributor

alice commented Mar 30, 2014

That sounds great! Looking forward to the pull request :)

@coderjz
Copy link
Contributor Author

coderjz commented Mar 31, 2014

There you go. I had some difficulty with grunt as this tool is new to me. Let me know if you need any further fixing.

BTW, saw your talk with Cameron at Fluent Conf 2014. Thanks for presenting this tool there, our team was not familiar with it and I brought it back home

@alice alice closed this as completed May 20, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants