Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should screens.currentScreen be [SameObject] ? #49

Closed
inexorabletash opened this issue Mar 16, 2021 · 6 comments · Fixed by #55
Closed

Should screens.currentScreen be [SameObject] ? #49

inexorabletash opened this issue Mar 16, 2021 · 6 comments · Fixed by #55
Assignees
Labels
question Further information is requested

Comments

@inexorabletash
Copy link
Member

Over in https://drafts.csswg.org/cssom-view/#dom-window-screen

partial interface Window {
    [SameObject, Replaceable] readonly attribute Screen screen;

This [SameObject] annotation indicates that even as the properties of the screen changes (e.g. via system changes, or the window being moved to a different display) the same Screen object is always returned, and updated live.

Should the same be true for the ScreenAdvanced object returned by Screens.currentScreen ? i.e. does it update live, with its id etc changing dynamically? Or does the object itself swap out?

I assume it should be [SameObject] but wanted to confirm

@inexorabletash
Copy link
Member Author

This may be relevant for the definition of when Screens.onchange fires. Right now the explainer says:

NEW: An event fired when 'screens' or 'currentScreen' changes.

That makes sense if the enumeration changes, but if currentScreen is always the same object, and like window.screen dynamically reflects the properties of the current screen, then what does it mean to say "'currentScreen' changes" - does it refer to the properties? If so, isn't that redundant with Screen.onchange ?

@quisquous
Copy link
Contributor

My informal reading of "currentScreen changes" is that it means that currentScreen.id has changed. In other words, that the page has been moved to another screen.

@michaelwasserman michaelwasserman self-assigned this Apr 23, 2021
@michaelwasserman
Copy link
Member

Yes, currentScreen should be [SameObject], so currentScreen.change should fire when properties of that screen change and when the window moves from one screen to another (which can be detected by changes to currentScreen.id).

Screens.change should only fire when a screen is added or removed from the screens array; it does not need to fire when a window moves between screens, and we should not need a separate EventHandler for that.

I'll update the explainer soon.

@michaelwasserman
Copy link
Member

Reopening this issue; @quisquous discovered some complications with making currentScreen [SameObject].

@quisquous
Copy link
Contributor

quisquous commented May 6, 2021

Coming back to this, I think making these objects [SameObject] overconstrains the problem in ways that I don't know how to resolve. This came up because I realized that tests were trying to assert that currentScreen exists in the screens array, but that creates problems when currentScreen is also [SameObject].

Say you have two screens, display 0 and display 1 in screens[0] and screens[1]. screens[0] === currentScreen and you move a window to the other display and so now display 1 is the current screen. I think that means that screens[0] would need to become a new object with display id 0, and then what used to be screens[1] is clobbered by currentScreen so that screens[1] === currentScreen is true (they were previously logically equal, but now need to be equal by reference).

But...what happens to the old screens[1] (or if you'd added a change event listener to it?) Also now if you add a change listener to screens[1] you're really adding it to currentScreen, and since it's [SameObject], it'll continuing firing when the current screen changes and not when screens[1] (by display id) changes.

I think the options really are:

(1) Don't make currentScreen be marked as [SameObject]. This is analogous to document.activeElement (which is also not [SameObject]). Add some additional event (e.g. currentscreenchanged) to represent the current screen changing. In this scenario, screens.includes(currentScreen).

(2) Mark currentScreen as [SameObject]. In this scenario, even if screens[0].id === currentScreen.id it is not true that screens[0] === currentScreen. No need for additional event listeners, as currentScreen.change means "whenever the current screen changes, whatever screen that happens to be" and screens[0].change means "whenever screens[0] changes".

I think I am inclined to go with the first option based on the document.activeElement example, but I feel like there's tradeoffs in both directions.

quisquous added a commit to quisquous/window-placement that referenced this issue May 14, 2021
* remove [SameObject] (see: w3c#49)
* split Screens.onchange into Screens.onscreenschange/oncurrentscreenchange
michaelwasserman pushed a commit that referenced this issue May 14, 2021
* remove [SameObject] (see: #49)
* split Screens.onchange into Screens.onscreenschange/oncurrentscreenchange
@michaelwasserman michaelwasserman added the question Further information is requested label Oct 30, 2021
@michaelwasserman
Copy link
Member

I think the current approach (not [SameObject]) makes sense for the reasons @quisquous explored above.
I'm closing this issue for now, but feel free to reopen as needed.

bradtriebwasser pushed a commit to bradtriebwasser/window-placement that referenced this issue Oct 19, 2022
* remove [SameObject] (see: w3c#49)
* split Screens.onchange into Screens.onscreenschange/oncurrentscreenchange
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants