trusted-types icon indicating copy to clipboard operation
trusted-types copied to clipboard

Add missing IDL changes to Parent and Child Node mixins from dom spec

Open lukewarlow opened this issue 1 year ago • 13 comments
trafficstars

Address #438

I haven't done the DOM parts change but have made a follow up issue for tracking that so it doesn't get lost

cc @mbrodesser-Igalia


Preview | Diff

lukewarlow avatar Feb 20 '24 13:02 lukewarlow

@lukewarlow could you please re-create this PR or refresh it (don't know if that's possible) so that diff and preview are generated (see https://github.com/w3c/trusted-types/issues/451#issue-2148990003, verified in https://github.com/w3c/trusted-types/pull/453)?

mbrodesser-Igalia avatar Feb 22 '24 14:02 mbrodesser-Igalia

Nice! that's working. Just had to rebase with main.

lukewarlow avatar Feb 22 '24 14:02 lukewarlow

Nice! that's working. Just had to rebase with main.

@lukewarlow could you please rebase the node-idl-updates branch too? Currently the generated Diff contains also changes for "execCommand" and perhaps more.

mbrodesser-Igalia avatar Feb 22 '24 14:02 mbrodesser-Igalia

Nice! that's working. Just had to rebase with main.

@lukewarlow could you please rebase the node-idl-updates branch too? Currently the generated Diff contains also changes for "execCommand" and perhaps more.

Or presumably your fork's main branch has to be rebased too.

mbrodesser-Igalia avatar Feb 22 '24 14:02 mbrodesser-Igalia

That seems to be a bug with the differ as this is fully rebased.

lukewarlow avatar Feb 22 '24 14:02 lukewarlow

That seems to be a bug with the differ as this is fully rebased.

To further simplify reviewing: couldn't this be a PR for the DOM spec? The diff would be simpler to consume. It has to move there eventually.

mbrodesser-Igalia avatar Feb 22 '24 14:02 mbrodesser-Igalia

That seems to be a bug with the differ as this is fully rebased.

To further simplify reviewing: couldn't this be a PR for the DOM spec? The diff would be simpler to consume. It has to move there eventually.

@lukewarlow WDYT about creating a draft PR for the DOM spec and link to that PR from a "Integration with the DOM" section in the TT spec?

mbrodesser-Igalia avatar Feb 22 '24 15:02 mbrodesser-Igalia

Issue with that is it relies on trusted types algorithms so I think should be moved when we eventually move everything else?

lukewarlow avatar Feb 22 '24 15:02 lukewarlow

Issue with that is it relies on trusted types algorithms so I think should be moved when we eventually move everything else?

Another draft relies on trusted types algos, too: https://github.com/whatwg/dom/pull/1247/files. As long as it's a draft PR, it's harmless anyway. There seems to be no advantage of keeping it in the TT spec.

mbrodesser-Igalia avatar Feb 22 '24 15:02 mbrodesser-Igalia

Another draft relies on trusted types algos, too: https://github.com/whatwg/dom/pull/1247/files. As long as it's a draft PR, it's harmless anyway. There seems to be no advantage of keeping it in the TT spec.

Okay yeah fair I forgot about that one. I'll move this across to a draft PR there

lukewarlow avatar Feb 22 '24 16:02 lukewarlow

I've opened https://github.com/whatwg/dom/pull/1258

lukewarlow avatar Feb 22 '24 17:02 lukewarlow

@koto I see you've updated IDL in your DOM spec PR should the IDL changes specifically be included in the trusted types spec too? Or should we just link out to the relevant place in the DOM spec (once merged)?

I want to ensure we have some unified place to come to for all trusted types changes but also understand we don't want to monkeypatch and duplicate effort.

I've updated this PR to the IDL change and a link to my PR only. But it'd be good to understand what we want the end goal to be. It'd be especially interesting to know what people think the plan should be for ongoing maintenence. Should we make a PR to HTML for example updating the relevant IDL with the new types?

lukewarlow avatar Feb 22 '24 18:02 lukewarlow

I think we should prioritize upstreaming all the integrations, as monkeypatching just isn't easy to work with. That includes DOM, DOM Parsing, HTML and WebIDL at the least.

koto avatar Feb 26 '24 17:02 koto

Closing this as these integrations will all be upstreamed.

lukewarlow avatar Mar 13 '24 14:03 lukewarlow