Open Bug 1925427 Opened 16 days ago Updated 1 day ago

WPT failIfNot utility-function doesn't handle the possibility that document.readyState might be "complete"

Categories

(Testing :: web-platform-tests, defect)

Product:
Testing
Component:
web-platform-tests
Type:
defect

Tracking

(Not tracked)

People

(Reporter: dholbert, Unassigned, NeedInfo)

References

Details

STR:

  1. Write a WPT that uses reftest-wait which runs some script in the <body onload="..."> handler.
  2. Try to use the failIfNot() utility-function in that script. In particular, try to make it fail by passing in a condition that evaluates to false.

EXPECTED RESULTS:
The test should fail with the provided message being written to the document.

ACTUAL RESULTS:
The failIfNot() message (its 2nd param) never gets appended to the document, even if the condition fails. That's because we're already at document.readyState of complete which failIfNot doesn't handle properly -- it expects us to either be in interactive (which is before complete), or else have a pending DOMContentLoaded event. In this case we are already past interactive, and DOMContentLoaded never arrives because it already fired before our script started running.

The current impl:
https://searchfox.org/mozilla-central/rev/b9e7c4300ba972a4c98bf463fc046e8cd9367175/testing/web-platform/tests/common/reftest-wait.js#27-39

function failIfNot(condition, msg) {
  const fail = () => {
    (document.body || document.documentElement).textContent = `Precondition Failed: ${msg}`;
    takeScreenshot();
  };
  if (!condition) {
    if (document.readyState == "interactive") {
      fail();
    } else {
      document.addEventListener("DOMContentLoaded", fail, false);
    }
  }
}

I suspect we want to change that readyState check to

    if (document.readyState == "interactive" ||
        document.readyState == "complete") {

jgraham, does that make sense?

Flags: needinfo?(james)
Component: CSS Parsing and Computation → web-platform-tests
Product: Core → Testing

Yes, I think so. I'd run the patch through try ofc, but in general I think the function was just designed on the assumption that you were running it synchronously in a script rather than in onload.

Flags: needinfo?(james)

The severity field is not set for this bug.
:jgraham, could you have a look please?

For more information, please visit BugBot documentation.

Flags: needinfo?(james)
You need to log in before you can comment on or make changes to this bug.