Examine tests which are commented out in <GlobalEventHandlers-onclick.html>

[mbrodesser-Igalia]

https://searchfox.org/mozilla-central/rev/b6718bf263ae780289da471f0ea35797a09e2f05/testing/web-platform/tests/trusted-types/GlobalEventHandlers-onclick.html#52-91

CC @fred-wang

[fred-wang]

These are the tests commented out:

// Trusted Type assignments via property access does not throw.
async_test(t => {
  window.onclickDone2 = t.step_func_done();
  let script = policy.createScript("window.onclickDone2();");
  let el = document.createElement('a');
  el.onclick = script;
  container.appendChild(el);
  el.click();
}, "a.onclick assigned via policy (successful Script transformation).");

// Unsuitable TrustedType assignments do throw.
async_test(t => {
  window.onclickFail3 = t.unreached_func();
  let script = policy_html.createHTML("window.onclickFail3();");
  let el = document.createElement('a');
  try {
    el.onclick = script;
    container.appendChild(el);
    el.click();
  } catch (e) {
    t.done();
  }
  assert_unreached();
}, "a.onclick assigned via an unsuitable policy.");

// So do plain test assignments.
async_test(t => {
  window.onclickFail4 = t.unreached_func();
  let el = document.createElement('a');
  try {
    el.onclick = window.onclickFail4();
    container.appendChild(el);
    el.click();
  } catch (e) {
    t.done();
  }
  assert_unreached();
}, "a.onclick assigned a test string.");

Basically, they verify that setting an event handler IDL attribute to a string or trusted type behaves the same as if we were setting the event handler content attribute.

For the latter (covered by the three first tests), the spec defines attributes change steps that are called from the DOM spec. The PR whatwg/dom#1268 modifies these callers, to add a preliminary conversion from Trusted types to strings.

For the former (covered by the tests commented out), the spec defines the steps to be run by the setter of an event handler IDL attribute, which eventually set eventHandler's value to the given value.

The eventHandler's value can either null, a callback object or an internal raw uncompiled handler. The last one only seems to be used by the "attribute change steps" mentioned above.

So I believe these uncommented tests do not correspond to anything in specs or open PRs and I'd suggest to remove them. If this is really something we want to support, probably the step 1.4.3 for the setter of an event handler IDL attribute should be modified to convert from Trusted types to strings and set the eventHandler's value to an internal raw uncompiled handler with proper value/location, and things should just work as desired.

Incidentally, more comments on these tests:

• There is a typo a.setAttribte
• The assert_unreached calls are incorrect, since the code is really executed after the try. I see this is causing assertion failures but the test still passes.
• Probably cleaner to just do a assert_throws_js(TypeError, () => { ... }) for cases where we expect exceptions to be thrown (and no need to use async test then).

I'll upload a PR.

cc @otherdaniel @lukewarlow @koto

[fred-wang]

The proposed changes are done in web-platform-tests/wpt#49910

I believe we can close this issue (I can't do it, my github account does not have proper permission). If we really think we should allow setting a (handler) property by trusted type, then we should probably have a separate spec issue about it and work on something in the lines of what I mentioned above.

[lukewarlow]

@koto would you be able to grant Fred the same permissions as me please.