-
Notifications
You must be signed in to change notification settings - Fork 779
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(target-size): support translucent elements within pseudo-stacking-context ancestors with implicit z-order #4351
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is the correct solution. The basic setup to reproduce the bug is that the parent needs to both be positioned without using z-index and use another CSS technique that creates a stacking context (transform
works as well). Then a child with opacity will cause the stack to think the parent is on top (without this change).
Tested with other styles besides using opacity and things like transform
or contain
on the child also show the same problem. So it's not just opacity this should account for but any CSS style that creates a stacking context.
<div id="one" style="width: 50xp; height: 50px; position: relative; contain: layout;">
<div id="two" style="width: 50xp; height: 50px; transform: scale(1)"></div>
</div>
<script>
axe.setup();
axe.commons.dom.getElementStack( document.getElementById('two') ) // [#one, #two]
</script>
Further testing of different setups shows that the issue seems to be how the stacking order is determined for positioned auto elements. Take the following example:
<style>
div {
width: 100px;
height: 100px;
}
</style>
<div id="1" style="background: red;">
<div id="2" style="contain: paint; background: rgba(0,0,255,0.5);"></div>
</div>
<div id="3" style="margin-top: -100px; background: green;">
<div id="4" style="background: yellow;"></div>
</div>
In the browser this creates a yellow and 50% blue background. Axe's element stack and stacking order is correct. The #2
element creates a stacking context so replaces the parent stacking context (which is what everything else uses), making it on top element.
// axe.commons.dom.getElementStack('#4')
[
"2",
"4",
"3",
"1",
"fixture"
]
// stacking order of every node in the element stack
[
[
{
"stackLevel": 0.1,
"treeOrder": 5,
"vNode": "#2"
}
],
[
{
"stackLevel": 0,
"treeOrder": 0
}
],
// ...
]
However if we change it to use positioned parents then everything is incorrect.
<style>
div {
width: 100px;
height: 100px;
}
</style>
<div id="1" style="position: absolute; top: 0; background: red;">
<div id="2" style="contain: paint; background: rgba(0,0,255,0.5);"></div>
</div>
<div id="3" style="position: absolute; top: 0; background: green;">
<div id="4" style="background: yellow;"></div>
</div>
In the browser this shows a solid yellow background. The parent elements both use fake stacking contexts so their stack orders are now 2 deep and positioned using POSITION_LEVEL
, but the #2
element is a stacking context so overrides the parent stacking context, thus creating the bug.
// axe.commons.dom.getElementStack('#4')
[
"2",
"4",
"3",
"1",
"fixture"
]
// stacking order
[
[
{
"stackLevel": 0.1,
"treeOrder": 5,
"vNode": "#2"
}
],
[
{
"stackLevel": 0,
"treeOrder": 0
},
{
"stackLevel": 0.3,
"treeOrder": 6,
"vNode": "#3"
}
],
[
{
"stackLevel": 0,
"treeOrder": 0
},
{
"stackLevel": 0.3,
"treeOrder": 6,
"vNode": "#3"
}
],
[
{
"stackLevel": 0,
"treeOrder": 0
},
{
"stackLevel": 0.3,
"treeOrder": 4,
"vNode": "#1"
}
],
[
{
"stackLevel": 0,
"treeOrder": 0
}
],
[
{
"stackLevel": 0,
"treeOrder": 0
}
],
[
{
"stackLevel": 0,
"treeOrder": 0
}
]
]
This makes it seem that the #2
element should be draw in the same stack level as the other nodes so it's drawn in DOM order (making it the bottom of the stack)
Note for when we next pick this up: Add test cases from #4629 and make sure the fix also covers those |
Draft until researching/testing more on:
Closes: #4350