UI doesn't reflect actual value of custom checkboxes

[Krinkle]

Tell us about your runtime:

• QUnit version: 3.0.0-alpha.2
• Which environment are you using? (e.g., browser, Node): browser
• How are you running QUnit? (e.g., QUnit CLI, Grunt, Karma, manually in browser): manually

What are you trying to do?

1. Define a checkbox:

QUnit.config.urlConfig.push('foo');

2. Tick it in the UI. The UI will reload.

What did you expect to happen?

• QUnit.urlParams.foo === true;
• QUnit.config.foo === true;
• Addressbar after reload includes ?foo
• Visually the checkbox remains checked after reload.

What actually happened?

The runner is parsing and reflecting the value correctly at runtime, but the UI isn't reflecting that. The checkbox stays off even though the address bar and test execution is correctly running with its actual enabled value.

• ✅ QUnit.urlParams.foo === true;
• ✅ QUnit.config.foo === true;
• ✅ Addressbar after reload includes ?foo
• ❌ Visually the checkbox remains checked after reload.

[Krinkle]

There is an ugly inter-dependency here:

• /src/core/browser/urlparams.js doesn't copy values from QUnit.urlsParams to QUnit.config until QUnit.begin(). This is because for it to be useful, people have to be able to add things to QUnit.config.urlConfig, which is generally done in user code between qunit.js and QUnit.begin(), e.g. an inline script, setup.js script, or via QUnit.on('runStart').
• HtmlReporter doesn't render the checkboxes until QUnit.begin() for the same reason. Although it of course has to be in the callback queue after the above.
• HtmlReporter wants its begin callback to be among the first, so that uncaught errors in user-defined begin callbacks don't prevent the UI from rendering.

Up until recently the above was in relative harmony based on the fact that /src/core/browser/browser-runner.js simply did these two in their appropiate order at https://github.com/qunitjs/qunit/blob/3.0.0-alpha.1/src/browser/browser-runner.js#L77-L79

  initUrlConfig(QUnit);
  // …
  QUnit.reporters.html.init(QUnit);

This fell apart over two commits:

1. 05e15ba (Add QUnit.config.reporters): This involved moving QUnit.reporters.html.init from being called unconditionally during qunit.js itself, to instead happening in QUnit.start() immediately before runStart/begin events.
   • To maintain support for reliable rendering of uncaught errors in user-defined begin callbacks, I added prioritizeSignal to "keep" the HtmlReporter among the first. I later realized this broke here, because prioritizeSignal only worked for QUnit.on(), not QUnit.begin().
   • To maintain accurate checkbox state, I figured it's fine to have HtmlReporter's handler before urlparams, because we don't actually call appendInterface() and build checkboxes until the "begin" event. I now realize this is non-sense and irrelevant, since urlparams does the same. The order still matters. This issues' bug should have started here, except I cancelled it out with not implementing prioritizeSignal correctly, so the bug wasn't actually introduced here.
2. 15acb36 (Early errors: HTML Reporter: Add support for displaying early errors #1786). This fixed prioritizeSignal to work for QUnit.begin(). I added a test case for early errors, and noticed commit 1 above broke that. This, of course, means it in effect, introduced this issue's bug.

[Krinkle]

The reason I found this bug, by the way, is because I'm working on rendering the UI immediately as part of qunit.js instead of waiting for window.onload. At least for cases where <div id="qunit"> already exists at that time, meaning, scripts tags are placed in the body after <div id="qunit">. This is something we've shown in examples for many years now, and I did that precisely for this future reason.

We could try to fix the execution order here, but maybe there's a way we can untangle this knot.

We use QUnit.config.urlConfig to decide which checkboxes exist, and we will have to keep doing that from a QUnit.begin() callback, because users must define their checkboxes there between qunit.js and the end of runStart. However, the internal order of the "begin" callbacks doesn't matter for that. Any placement is fine, as we'll always know which checkboxes need to be displayed from a "begin" callback. This part is stable.

So we reliably know which checkboxes need to be displayed, regardless of internal ordering of "begin" callbacks. What we don't reliably know is the value of those checkboxes.

At glance, it seems like we can avoid all this drama by simply reading the intended value of the checkbox state from QUnit.urlParams[id] instead of QUnit.config[id]. QUnit.urlParams is populated unconditonally as part of qunit.js and is always available. It's population isn't deferred or configurable. It will contain whatever location.search contains, including values for query parameters that aren't formally registered. That's okay so long as we only read from known keys. The value is the value, right?

Not exactly. The reason we read from QUnit.config is for checkboxes that represent core features such as "No try-catch" and "Hide passed". These can be configured via preconfig and should reflect the configured value, not per-se from url parameters. Additionally, url parameters for built-in features sometimes end up discarded as invalid. E.g. custom=foo will reflect as custom="foo", casted to boolean true. But hidepassed=foo would be considered invalid and ignored in favour of QUnit.config (either defaults, preconfig, or explicit QUnit.config assignment).

What we could do, is change HtmlReporter to read from QUnit.config[id] where possible, and if undefined, read from QUnit.urlParams[id]. That's effectively all urlparams.js does anyway.