- 
                Notifications
    
You must be signed in to change notification settings  - Fork 3.5k
 
Description
I happened to run across an issue with a WPT today where a test doesn't seem to be doing what it was meant to be doing... (or alternately, it's got some inactive-copypasta that should be removed).
Here's the test:
https://wpt.live/css/css-contain/content-visibility/content-visibility-046.html
@vmpstr could you take a look when you get a chance? You're credited as the test-author in the test's source code and in https://chromium-review.googlesource.com/c/chromium/src/+/2236919 / https://issues.chromium.org/issues/40133738 where this WPT was added. I'm hoping you can recall or intuit what this test was supposed to be doing. :)
Here's an excerpt of the source in question, with issues noted below:
<style>
[...]
.auto {
  content-visibility: auto;
}
</style>
<div id=container class=auto>
  Test passes if you can see this text and a green box.
  <div id=child></div>
</div>
<script>
function runTest() {
  document.getElementById("target").classList.add("auto");
  requestAnimationFrame(takeScreenshot);
}
window.onload = requestAnimationFrame(
  () => requestAnimationFrame(takeScreenshot));
</script>Notable issues:
(1) The runTest() function is never called.  Instead, we just directly call takeScreenshot when the page loads (in a rAF callback).
(2) It's a good thing runTest() doesn't get called, because it references an element with id target, and there is no such element.
(3) The runTest function (if it ran) is trying to add the auto class to an element, to give it content-visibility:auto. The testcase happens to already have the auto class present on the element with id=container.  Maybe (?) the intent here was for that class to be added to container dynamically?
In any case, it seems like we should either remove all the scripts and the reftest-wait usage here (since we're not actually doing anything dynamic), or we should fix up runTest to do whatever it was supposed to be doing (e.g. dynamically adding auto to container), and then change the onload handler to invoke it.