Pseudo element contrast failure too invasive?

[cchaos]

👋

I've run into a situation where :nth-child style pseudo selectors are wrongly being identified as pseudo elements and therefore, can't assume contrast acceptance. I've tracked down to this section here, but it does seem to only be checking for :before and :afters. Maybe something in the function being called?

axe-core/lib/checks/color/color-contrast-evaluate.js

Lines 103 to 116 in f2f1537

	while (parentNode) {
	if (
	hasPsuedoElement(parentNode, ':before') ||
	hasPsuedoElement(parentNode, ':after')
	) {
	this.data({
	messageKey: 'pseudoContent'
	});
	this.relatedNodes(parentNode);
	return undefined;
	}
	parentNode = parentNode.parentElement;
	}

Anyhow, here's also a screenshot showing the run through using the Chrome plugin and the result:

[myasonik]

In case it helps someone investigate, I found this change happened in this PR: #2613

Which was released in 4.1.0 and then got rolled out to the browser plugins with 4.1.1

[straker]

Interesting. Is there a live site I could look at? It looks like at some point in that elements parent chain a :before or :after is declared. Looking at the related node it belongs to the .euiSideNavItem__items class

[myasonik]

The example above is from https://eui.elastic.co

[straker]

The before-mentioned class has this css on it which is causing the message:

.euiSideNavItem__items:after {
    position: absolute;
    content: '';
    top: 0;
    bottom: 12px;
    width: 1px;
    background: #D3DAE6;
    left: 0;
}

Since the content is '' it is being flagged as having a pseudo element with background color. However, another css then makes the pseudo element display: none which hides it, which we don't take into account.

[cchaos]

🤦 Good catch on there actually being a pseudo element. The element just also happened to have a parent selector that uses a pseudo selector.

The pseudo element is the tiny little border the connects our text to an indent line which is hidden until the previous item opens.

So then I guess the question is, is the current "failure" too invasive? This is a trick we use a lot in CSS to position, primarily graphical (non-critical) elements and the errors are then overwhelming. Is it possible to ignore pseudo elements when their content is an empty string?

[straker]

At the moment no. The code was added to help prevent false positives when an element would fail color-contrast even though a pseudo element changed the background color that should have caused it to pass. Since there's now way to figure out what the pseudo element's size is (there's no way to get the boundingClientRect) we decided to settle on just informing the user when a psuedo element is used.

@WilcoFiers maybe this could be an option for color-contrast?

[craigkovatch]

Count me as a vote in agreement that this warning has way too high of a potential for false positive.

@straker could this test be improved in the case that the pseudo-element has defined dimensions? AFAIK it is possible to getComputedStyle on a pseudo. Could get the boundingClientRect of the element it belongs to and then do the math from there. Asking because we're seeing this warning on e.g. our checkbox control, where the checkbox image is added via pseudo-element. It's on top of an opacity: 0 native browser checkbox, and doesn't intersect with the text at all.

    '&:before': {
      backgroundColor: 'white',
      border: `1px solid ${IconFillColors.Rest}`,
      borderRadius: 1,
      boxSizing: 'border-box',
      content: `'\\200B'`, // zero-width space
      display: 'inline-block',
      height: BoxDimension,
      position: 'absolute',
      top: 3,
      width: BoxDimension,
    },
    '&.checkbox.checked:before': {
      content: checkboxCheckedSvg(IconFillColors.Rest),
    },

[straker]

@craigkovatch We've thought about doing the math, but I'm afraid that would cause more issues than just flagging the element for Needs Review. To determine an accurate position, we'd have to:

1. determine which ancestor element is used as the anchoring element (who has the first position CSS as it doesn't have to be the element the pseudo is attached to)
2. get it's bounding client rect
3. determine the pseudo element's size and position (is it using top and bottom? A set width? Negative margin?)
4. calculate any CSS transforms (this is the most complicated part)

There's probably more steps, but this gives the general idea. Until browsers have an API to return the boundingClientRect of the pseudo element, I'm afraid this is the best we can do to avoid false positives on color contrast.

[craigkovatch]

@straker I would like to register again the strong objection to the false negatives that this approach generates. We use pseudo elements to render custom images for styled checkbox and radio inputs, which is fairly common practice across the web (see my screenshot in previous comment). Since upgrading our Axe version, we are now getting a plethora of these false failures. This is causing many hours of wasted engineering effort by those who are unaware that the failures should be ignored. Please reconsider this change, or offer a much more robust way of skipping/disabling it. It's bad enough that I'm recommending we (Tableau) actually roll back our version of Axe to avoid these failures.

[dylanb]

@craigkovatch we are implementing some UI changes to make this easier to handle. The end result will be that you only see the review issues when you perform manual testing and we will have functionality to help resolve the issues easily.

[craigkovatch]

@dylanb we’re currently getting these results using the JS library. Do the changes you’re referencing apply in this scenario?

[straker]

@craigkovatch the UI changes wouldn't affect the js library. We'll add an option to disable the pseudo check of color-contrast so you can turn off these warnings if you want.

@clottman for this one, let's check in the pseudo algorithm the display and visibility of the element, if it's none or hidden (respectively) we shouldn't flag it as having a pseudo element. Additionally, let's add an option to disable the pseudo check entirely. Let's call it ignorePseudo to match the other options we have, and set it to default to false.

[craigkovatch]

We'll add an option to disable the pseudo check of color-contrast so you can turn off these warnings if you want.

Please and thank you! :)

[straker]

Ok. We added the option and it should be available in the next feature release (coming in the next few weeks hopefully). Do note though that turning off the option may result in a Needs Review item being marked as a failure, so you'll need to watch out to see what it will change for your tests.

[craigkovatch]

@straker thank you very much for the attention on this! Your comment on needing to watch out what it may change in our tests is concerning to me though. Please bear with me the upcoming wall of text 😅

The specific situation I'm in is this: my team writes and maintains the web UI library that the rest of our company builds their UI modules with. They consume our library as an npm dependency, so there's no shared repository. There are well over 100 other packages that are consuming our library, and an increasing number of them are using Axe as part of their test suite in order to discover potential a11y issues before shipping.

Those individual teams and packages own their use of Axe, so I don't have the ability (nor the capacity) to monitor or update any failures that might occur. But the failures that are occurring in this case are occurring in their test runs against the code I do own, e.g. checkbox and radiobutton controls that use pseudo-elements to paint customized visuals on top of a native (and transparent) <input> element. (This particular approach is pretty common practice for styling native HTML inputs; we didn't invent it and it's not a crazy insane edge case.)

I'm very appreciative for your response on this issue. But I'm worried that I'm going to be back to the same place in the end -- other teams will be getting test failures that lead to major production bottlenecks against code they don't own, and that will cost lots of people lots of time as they try to diagnose.

I think there's a strong possibility here that I'm just not understanding the change that's been made, and maybe everything is gonna be fine.gif 😌 but please let me know if you think the change that's been made will allow us to handle this scenario in a straightforward way.

[straker]

I guess it depends.

In the fix we made sure to ignore pseudo elements that are hidden (display: none or visibility: hidden). If your elements did this then they won't be marked as even having pseudo elements so you should be fine.

The main concern is a specific case of where you have a pseudo element and that element could contribute to the background color (which we can't know for sure because there's no API we can ask). These elements would be marked as Needs Review with the current code, but setting the option we added it could be that these elements move from Needs Review to either a false positive or false negative. However, the pseudo element in question has to follow a specific pattern of css:

element::before, element::after {
  content: '';
  position: absolute;
  background-color: 'red';
}

Your other option is to use a techniques like https://adrianroselli.com/2017/05/under-engineered-custom-radio-buttons-and-checkboxen.html or https://www.sarasoueidan.com/blog/inclusively-hiding-and-styling-checkboxes-and-radio-buttons/ which do not fail the color-contrast check

[craigkovatch]

@straker Respectfully, it's not really tractable for us to start completely avoiding the use of pseudo elements throughout our UI in order to avoid triggering a false error in Axe. IMO "rewrite your implementation" should not be the philosophy of any testing library absent extremely compelling circumstances. (Also while the issue I'm discussing is currently about checkboxen/radiobuttons, it could easily come up in another context in which the techniques you linked aren't applicable.)

I've pasted the relevant code for this exact use case below. (If it would be helpful, you can see the actual component in question live at https://tableau.github.io/tableau-ui/docs/index.html#checkbox) TL;DR: hide the native <input> with opacity: 0, use a pseudo element to absolutely-position a box and paint an SVG checkmark on top of the transparent <input>, and wrap that all in a <label> element to provide the associated text. We only use this control on white backgrounds, so there are no color contrast issues.

Could you please clarify if this would meet the "specific pattern of css" requirement you mentioned? (In particular, your example code includes an empty string for content, which I believe will cause any pseudo to simply not appear at all -- this is why we use \u200B ourselves.) If not, I would like to please further iterate on a solution here, so that we don't have to stay on an obsolete version of Axe to avoid these false positives.

<label class="f13vtjdj checkbox checked enabled">
    <input type="checkbox" class="f7yikj" /><div class="f1u2kjnq">Enabled</div>
</label>

.f13vtjdj {
    color: #333;
    cursor: default;
    display: inline-flex;
    font-size: 12px;
    line-height: 20px;
    position: relative;
    user-select: none;
}
.f7yikj {
    box-sizing: border-box;
    flex-shrink: 0;
    height: 14px;
    margin: 0;
    margin-top: 3px;
    opacity: 0;
    padding: 0;
    pointer-events: none;
    width: 14px;
}
.f13vtjdj:before {
    background-color: white;
    border: 1px solid #666;
    border-radius: 1px;
    box-sizing: border-box;
    content: '\200B';
    display: inline-block;
    height: 14px;
    position: absolute;
    top: 3px;
    width: 14px;
}
.f13vtjdj.checkbox.checked:before {
    content: url(data:image/svg+xml,...);
}
.f1u2kjnq {
    cursor: default;
    margin-left: 6px;
    overflow: hidden;
    text-overflow: ellipsis;
    user-select: none;
}

[straker]

In your case it looks like using the option should be fine. The checkbox doesn't overlap the text (which is the part we check for color contrast) so it shouldn't result in any false positives. If the pseudo element were to ever extend into the text, then it could cause some problems.

An empty content just means it's an empty string, but the pseudo element it will still appear in the DOM.

[craigkovatch]

Fabulous. Thanks!!!

[padmavemulapati]

Validated with the latest codebase, develop branch,
observed the color-contrast Pseudo element failure behaviour:
test script and "Needs review" and "violations" list as in the screenshots

<style>.foo { position: relative; } .foo:after { content: ""; position: absolute; width: 100%; height: 100%; background: red; }</style>
        <div id="background" class="foo"><p id="target" style="#000">Content</p></div>
        <style>.foo { position: relative; } .foo:before { content: ""; position: absolute; width: 100%; height: 100%; background-color: red; visibility: hidden; }</style>
        <div id="background" class="foo"><p id="target" style="#000">Content</p></div>
        <style>.foo { position: relative; } .foo:before { content: ""; position: absolute; width: 100%; height: 100%; background-color: red; display: none; }</style>
        <div id="background" class="foo"><p id="target" style="#000">Content</p></div>