Do not throw if the attribute setter is called with no arguments

[arai-a]

Do not throw if the attribute setter is called with no arguments

This is for #1497 .

This changes the attribute setter not to throw if it's called with no arguments (this can happen only if the setter function is extracted from the property descriptor and directly called), and instead treat it as if undefined is passed.

No browsers had been following the previous behavior, and the updated behavior matches WebKit.

• At least two implementers are interested (and none opposed):
  • WebKit (The attribute setter operation mismatch between browsers for the arguments count #1497 (comment))
  • Chromium (The attribute setter operation mismatch between browsers for the arguments count #1497 (comment), The attribute setter operation mismatch between browsers for the arguments count #1497 (comment))
  • Gecko (The attribute setter operation mismatch between browsers for the arguments count #1497 (comment) , and patch is ready in https://bugzilla.mozilla.org/show_bug.cgi?id=1974950)
• Tests are written and can be reviewed and commented upon at:
  • The patch part 2 in https://bugzilla.mozilla.org/show_bug.cgi?id=1974950
• Implementation bugs are filed:
  • Chromium: https://issues.chromium.org/issues/429221315
  • Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1974950
  • WebKit: (Already matches the updated behavior, and it's passing the WPT added in the above)
  • Deno: Do not throw if the attribute setter is called with no arguments denoland/deno#30167
  • Node.js: (unaffected)
  • webidl2.js: (unaffected)
  • widlparser: (unaffected)
• MDN issue is filed: (extracting the setter function and directly calling it shouldn't be done in regular usage, and there won't be any need to describe this behavior in MDN)
• The top of this comment includes a clear commit message to use.

---

Preview | Diff

[arai-a]

Thank you for reviewing!

I have some questions:

• what should I do for Deno and Node.js ? I'm not sure whether and how they're affected.
• for wpt, is it okay with landing it as Firefox's patch, which would be automatically merged? or should I directly create a PR to wpt repository and ask for review there?

[annevk]

@lucacasonato and @jasnell should be able to comment for Deno and Node.

Changing WPT as part of Firefox's workflow is fine. Though ideally the window between the specification change landing and the WPT PR is landing is small.

[domenic]

LGTM.

I am somewhat doubtful Deno and Node implement the specs so precisely here as to have run into this bug :)

[lucacasonato]

We do implement it precisely, and do throw when arguments.length is 0, but it seems ok from our side to change this.

[arai-a]

I've checked some Node's WebAPIs, but so far I cannot find any affected cases.
arguments.length is checked by constructors and operations, but not by setters.
So I'll mark it unaffected for now.
Please correct me if I'm wrong.

[arai-a]

Changing WPT as part of Firefox's workflow is fine. Though ideally the window between the specification change landing and the WPT PR is landing is small.

Thanks!
The Firefox's patch stack (https://bugzilla.mozilla.org/show_bug.cgi?id=1974950) is ready to land, and the WPT PR will be created/merged when it's landed.

And now bugs for all affected implementations are filed.
How should we coordinate the landing? The spec PR goes first and then the Firefox patch, with the WPT PR?

[annevk]

Sounds good!

Note that if you have to revert the Firefox patch for some reason, it'd be great if you could keep the WPT coverage intact.

[arai-a]

web-platform-tests/wpt#53894 is created for the WPT