Definition of valid CropTarget might be too loose

[youennf]

A CropTarget is defined in terms of browsing context which can potentially outlive navigations.
I believe it would be best handled in terms of active document of the top level browsing context instead.
This will ensure that if a CropTarget of a previous document of the same browsing context is used, it is not considered valid.

[jan-ivar]

Agree the current definition is flawed. Editorially, I also don't think we should outsource "validity" to any sub-algorithm like this, because what's valid when something is called has a lot to do with when and where that call is made. So I prefer cropTo spell out its input validation steps explicitly and synchronously.

The cropTo validation steps seem to me to be (unrelated to produceCropTarget):

1. If the target's element is no longer around, bail
2. If that element is no longer in a document, bail
3. If that document is not this (track)'s captured top-level browsing context's active document, or not an active document in one of this (track)'s captured top-level browsing context's nested browsing contexts, bail.

[eladalon1983]

A CropTarget is defined in terms of browsing context which can potentially outlive navigations.

Sure, if we have a way of defining things in a better way, let's do so. But note that allowing a CropTarget to outlive navigation would effectively mean cropping the track mutes it, as no additional frames would be produced that have >0 pixels. So not a security concern, whether we reject cropTo() or make it "mute" the track.

1. If the target's element is no longer around, bail

That seems fine.

2. If that element is no longer in a document, bail

Can you explain the distinction between this and the previous?

If that document is not this (track)'s captured top-level browsing context's active document, or not an active document in one of this (track)'s captured top-level browsing context's nested browsing contexts, bail.

Seems reasonable on first glance.

[youennf]

So I prefer cropTo spell out its input validation steps explicitly and synchronously

Note though that validating that a CropTarget is still referencing an existing element cannot be guaranteed synchronously.
Validating that it is part of the right top level document seems ok to do synchronously.

[eladalon1983]

So I prefer cropTo spell out its input validation steps explicitly and synchronously

Note though that validating that a CropTarget is still referencing an existing element cannot be guaranteed synchronously. Validating that it is part of the right top level document seems ok to do synchronously.

It's not immediately obvious that we would even want to expose that it's referring to an existing, non-garbaged collected Element, as that exposes information about garbage-collection. I think it's best to check (asynchronously) that the CropTarget was minted for an Element that was part of the same "tab". (That is, in a browsing context that is a descendant of the top-level document's browsing context yada yada...) What's happened to the Element since the CropTarget was minted is not relevant.

[youennf]

I did an initial stab at it based on using a CropTarget element internal slot at youennf@b8a9cec

[eladalon1983]

Could you please (1) update this PR in light of the many recent PRs that we've landed and (2) send it for review as an official PR?