-
Notifications
You must be signed in to change notification settings - Fork 799
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
Ligature color-contrast checks result as incomplete #1906
Comments
No customer-facing docs required. |
Verified this issue with the latest axe-coconut, I am still seeing the color-contrast issue in the "needs review" not in the voilations. |
So the issue is our color-contrast check returns incomplete for the node because the content is too short: "Element content is too short to determine if it is actual text content." Not sure we can do anything about it as the code is working as intended. |
As per @straker , nothing else here to do , so closing the issue |
@padmavemulapati and/or @straker would it be possible to check if the node only contains ligature's and then not care about the content length? Seems like a big miss if all lone ligature's are unchecked for color contrast, since it's quite common to use ligature's for icon's, especially ✔️ or 🚫 type icon's who might be green or red and should really be checked for color contrast. |
@KyleBastien , I ran axe and axe-coconut on the given test file, when I observed with color-contrast analyzer, realised that red colored ligature is having low color-contrast ratio than the standard ratio.. so that issue is coming under needs review, which is expected as per the ticket. Please find below screenshots and please let me know if anything else need to be taken care inorder to evaluate this ticket. |
@padmavemulapati I see that it's marked as needs review, but as per the original post on this issue, it should be a violation, not a incomplete, similar to all other text with contrast ratio issues. From the screenshot you shared, it still seems like it's marking as incomplete. If the length of the content is what is marking it as incomplete, then ligature's by the themselves shouldn't be bound to length, because they can contain just icon's by themselves and in those cases color contrast checked are very important. |
@KyleBastien So ✔️ or 🚫are not ligatures but emojis, which we cannot check for color-contrast at the moment (they have their own colors / look based on the platform, etc.). Text ligatures such as "AE" or "fi" should rarely be used on their own. The other case is a ligature icon font such as Material Icon where the text "file" is replaced by an icon. These should be correctly checked for color contrast. |
@straker I know ✔️ and 🚫are emoji's, I was trying to reference them as examples of types of icon ligatures that are frequently used by themselves.
This doesn't appear to be the case, or there is a bug with this. See this example, which uses the Office Fabric Ligature Icon's: When I run axe-coconut on this I see this: But, when I run a color contrast tool on this, this doesn't pass: Let me know if I'm doing something wrong in this test, but this doesn't seem to check for color contrast on ligature icon's. |
So we filter unicode icons (specifically non-bmp range unicode characters) from being run through the color-contrast check if that's all the text includes. I believe we filter them since there is no guarantee the unicode icon will display properly at that range. However, in this case the unicode character is being used as a key for the fonts ligature symbols, but I'm not sure we can detect that. |
@straker Can you clarify more on this?
When you say display properly here, what do you mean? |
@straker Ahh thanks for clarifying. Would it be possible to turn off skipping over Unicode characters via a config to axe-core? We won't be testing ligature's on OS's that don't render them, but I also understand that might be specific to our use case. So I'm wondering if a config might be possible here? |
@KyleBastien Sure. Would you be interested in writing a PR for it? |
@straker Sure! Could you point me towards where the logic is that currently skips over Unicode characters? And also where to add a new config property? |
Awesome! So there will be 2 things that need to happen. 1st, we need to stop filtering unicode and 2nd, we need to have an option to ignore the length requirement (as a single unicode will still be "too short"). The first part is done in the color-contrast-matcher function (the For the second part, options can be added to individual checks (what a rule runs). You can see an option for aria-roledescription as an example. So you'll need to add an Lastly, you'll need to use the option in the check function itself. The check function is passed an |
@straker I guess I was thinking of something slightly different for the length requirement piece that might not need an additional flag. Basically, as I understand from above, the length requirement doesn't come into play for ligatures when those ligatures are by themselves, since Material Icon ligatures appear to be checked correctly for color contrast. Am I correct there? Could we treat unicode characters the same, if the "filterUnicodeCharacters" is set to false? i.e. If it is just a unicode character by itself, it's probably an icon therefore do a color contrast check independent on length? Also, if I'm wrong in my understanding of how ligatures are checked, what do you think of making the flag something like "ignoreLigatureLength", so this is more scoped to just ligature checks and not ignore length checks for everything? |
Material Icon uses text as a ligature key so the string "android" is replaced by the Android icon. That's why it works for our color contrast because it uses normal text in the DOM. @WilcoFiers what do you think about @KyleBastien suggestions? |
Adding two new config flags to color-contrast so it can check unicode character based icons. Needs tests. Related to dequelabs#1906.
I took a first shot at the implementation you described, using two config values. If you could take a look and let me know if you think this is along the right lines, that would be greatly appreciated! |
Adding two new config flags to color-contrast so it can check unicode character based icons. ignoreUnicode, defaults to true and retains the behavior of ignoring all unicode characters when doing color contrast. This can be turned off to start checking unicode characters for color contrast. ignoreLength, defaults to false and retains the behavior that single character nodes do not contain enough information to say whether or not they have color contrast issues. This can be turned on to ignore this length check and always check if a node has color contrast issues. Fixes issues described in dequelabs#1906.
@straker Happy New Year! If you get the opportunity to take a look @ the change I implemented above, I would greatly appreciate it! Thank you! |
@KyleBastien Happy New Year to you as well! Looking over the changes it looks like everything's working. I don't have any immediate concerns from my quick look, so feel free to open a PR. |
Adding two new config flags to color-contrast so it can check unicode character based icons. ignoreUnicode, defaults to true and retains the behavior of ignoring all unicode characters when doing color contrast. This can be turned off to start checking unicode characters for color contrast. ignoreLength, defaults to false and retains the behavior that single character nodes do not contain enough information to say whether or not they have color contrast issues. This can be turned on to ignore this length check and always check if a node has color contrast issues. Fixes issues described in dequelabs#1906.
@straker Thanks! I opened the PR! |
Adding two new config flags to color-contrast so it can check unicode character based icons. ignoreUnicode, defaults to true and retains the behavior of ignoring all unicode characters when doing color contrast. This can be turned off to start checking unicode characters for color contrast. ignoreLength, defaults to false and retains the behavior that single character nodes do not contain enough information to say whether or not they have color contrast issues. This can be turned on to ignore this length check and always check if a node has color contrast issues. Fixes issues described in dequelabs#1906.
Adding two new config flags to color-contrast so it can check unicode character based icons. ignoreUnicode, defaults to true and retains the behavior of ignoring all unicode characters when doing color contrast. This can be turned off to start checking unicode characters for color contrast. ignoreLength, defaults to false and retains the behavior that single character nodes do not contain enough information to say whether or not they have color contrast issues. This can be turned on to ignore this length check and always check if a node has color contrast issues. Fixes issues described in dequelabs#1906.
…lags. Adding two new config flags to color-contrast so it can check unicode character. Sometimes ligature based icons utilize unicode characters as stand-in's, so these flags become useful to be able to do color-contrast checks on these icons. ignoreUnicode, defaults to true and retains the behavior of ignoring all unicode characters when doing color contrast. This can be turned off to start checking unicode characters for color contrast. ignoreLength, defaults to false and retains the behavior that single character nodes do not contain enough information to say whether or not they have color contrast issues. This can be turned on to ignore this length check and always check if a node has color contrast issues. Resolves discussion in dequelabs#1906
…lags. (#1969) Adding two new config flags to color-contrast so it can check unicode character. Sometimes ligature based icons utilize unicode characters as stand-in's, so these flags become useful to be able to do color-contrast checks on these icons. ignoreUnicode, defaults to true and retains the behavior of ignoring all unicode characters when doing color contrast. This can be turned off to start checking unicode characters for color contrast. ignoreLength, defaults to false and retains the behavior that single character nodes do not contain enough information to say whether or not they have color contrast issues. This can be turned on to ignore this length check and always check if a node has color contrast issues. Resolves discussion in #1906
Expectation: Ligature with failing contrast ratios should result as violations.
Actual: Ligature with failing contrast ratios result as incomplete.
Motivation: Ligature with inaccessible contrast should be considered violations, similar to how text color-contrast issues are violations.
Example code:
axe-core version: 3.4.0
cypress-axe: 0.5.1
cypress: 3.6.1
The text was updated successfully, but these errors were encountered: