WICG / document-picture-in-picture

https://wicg.github.io/document-picture-in-picture/
Other
56 stars 9 forks source link

Add disallowPositionReuse #119

Closed liberato-at-chromium closed 2 weeks ago

liberato-at-chromium commented 5 months ago

A UA is free to select a position and size for a document PiP window that does not conform to the size requested by the site with requestWindow(). A typical use is to allow user resizes to take precedence if a PiP window is closed and reopened later by the same site, so that the user's preference takes precedence.

This change adds the disallowPositionReuse flag to notify the user agent that this PiP window is semantically unrelated to the previous one, so ignoring the most recent user-selected size might be more appropriate, as if this were the first PiP window opened by the site.

liberato-at-chromium commented 5 months ago

I think it could be useful to include a note after steps 12 and 13 of the requestWindow(options) method steps, saying something along the lines of "if the website says disallowPositionReuse, then you might want to not do that"

+1 to adding a note. However, 12 and 13 don't mention bounds caching at all. i'll add some text that disallowPositionReuse may be considered as part of this decision, but it's hard to be more specific without bigger changes to 12,13. please let me know if you'd like me to try making those bigger changes.

steimelchrome commented 5 months ago

Note: https://wicg.github.io/document-picture-in-picture/ spec is still showing allowReturnToOpener, not disallowReturnToOpener. @steimelchrome I assume https://github.com/WICG/document-picture-in-picture/actions/runs/8284843825/job/22671289688 is the culprit one. Can you fix it?

Good catch. I re-ran the failed job and it should be fixed now

eladalon1983 commented 4 months ago

nit: Please consider if rephrasing in the positive might be clearer. For example, usePreviousWindowBounds, defaulting to true. Or possibly preferPreviousWindowBounds, indicating to the Web developer both that it's a hint the UA could ignore, as well as that things might have changed in the meantime to the point of inability to accommodate (e.g. change of screens and/or resolution).

beaufortfrancois commented 4 months ago

nit: Please consider if rephrasing in the positive might be clearer. For example, usePreviousWindowBounds, defaulting to true. Or possibly preferPreviousWindowBounds, indicating to the Web developer both that it's a hint the UA could ignore, as well as that things might have changed in the meantime to the point of inability to accommodate (e.g. change of screens and/or resolution).

Defaulting to true was avoided on purpose. See https://github.com/WICG/document-picture-in-picture/issues/115#issue-2185223432

eladalon1983 commented 4 months ago

Defaulting to true was avoided on purpose.

Would using an enum be a valid way of sidestepping that? windowBoundsPreference set to either "previous" or "new" perhaps?

liberato-at-chromium commented 4 months ago

Would using an enum be a valid way of sidestepping that?

An enum would let us specify a default value, but it feels like cheating a bit if it's just a boolean in disguise. I don't know if we ever plan to have multiple options for this. windowPlacementStrategy = { preferInitial, preferMostRecent }?

indicating to the Web developer both that it's a hint the UA could ignore

I agree that the original name disallow... was a little too strong for a hint. ignore... might also suffer from this. prefer is a lot clearer in this regard, in my opinion. I wouldn't mind naming this with something that starts with prefer.

eladalon1983 commented 4 months ago

but it feels like cheating a bit if it's just a boolean in disguise

If the guidance were against booleans in an of themselves, yes, that'd have been cheating. But since the problem stated was with auto-conversion of undefined to false rather than to true, I think an enum would be fine. Wdyt?

liberato-at-chromium commented 4 months ago

i'm fairly new at spec-land, so i'll not have too strong an opinion on booleans vs enums.

If we go the enum route, windowPlacementStrategy sgtm.

If we stay with the boolean, maybe preferInitialWindowPlacement in place of ignorePreviousWindowBounds given that it's more clearly a hint than a requirement.

@beaufortfrancois @steimelchrome WDYT?

eladalon1983 commented 4 months ago

If precedents are useful, btw, here is a case where we have four enums (as of the time of writing) that are all just "include"/"exclude" - so basically an enum.

liberato-at-chromium commented 3 months ago

After some thought, i decided to go with boolean preferInitialWindowPlacement. Keeps false as the default.

I considered an enum, but couldn't come up with a case where we'd want more than two options (640k is enough!), and prefer... seemed pretty natural.

As an aside, I expect that we'll have another option to describe how initial window placement should work ("upper left"), so "prefer initial" as a boolean fits pretty well into that.