[pull] master from jitsi:master - #837
Merged
Merged
Conversation
* feat(multi-screen): in-app triggers for the second screen - add a "Show on second screen" entry to the local, remote and fake participant menus, and a hover icon on the screenshare and shared-video tiles, which have no menu of their own - toggle the same trigger to take a source off its screen again, which is the only way to close an in-app second screen from the meeting window - pick the target window by filling a free external screen first, then reusing the least recently targeted in-app window; a window the embedder opened through the external API is counted but never taken over - cache the ScreenDetails so a send stays in the click's own task and keeps its user activation, and pre-load them on CONFERENCE_JOINED when the permission has already been granted, which never prompts on its own - notify the user when a send fails; until now a blocked popup or a denied permission was reported only to the embedder, so an in-app click that failed did nothing observable - resolve a screenshare named by its virtual screenshare participant, so the right one is shown when several are live at once * fix(multi-screen): drop the unreachable second-screen load fast path - remove the already-loaded check in awaitSecondScreenLoad: the caller always opens with the shell URL and navigating a reused named context loads it again, so the only document that could match is a stale outgoing one, which carries the marker too and would have the handle built on it just before it is replaced - correct the in-flight guard comment, which claimed nothing is lost when a concurrent request is dropped; that holds on the success path only, since a failed open removes the entry the dropped request had written * fix(multi-screen): address the review on the in-app triggers - Derive which screens are occupied from where the second-screen windows actually are, not from the screenId their entry was opened with. That id only records intent at open time: the browser renumbers ScreenDetails.screens when a display comes or goes, a live window is never re-placed, and an entry can carry no id at all, so it drifted from reality three different ways and a send could open a window fullscreen on top of a live one. Occupancy now reads each window's own position, frees the screen of a window closed by hand, and only falls back to the stored id for an entry whose window has not opened yet. - Keep the stored screenId when a setSecondScreen names no screen, so an embedder re-sourcing a window it placed explicitly no longer clobbers it. - Open the window before obtaining the screen details rather than after. On a profile that has not granted window-management yet the prompt sat between the click and window.open, and Chromium expires transient user activation after 5s, so a user who read the prompt before pressing Allow had their popup blocked. The window is now opened unplaced in the click's own task and moved onto its screen once the permission is answered. - Close an in-app second screen when what it was sent has left the meeting: a screenshare that stops, a participant who leaves, a shared video that ends. Each trigger lives on the thing it sends, so the only toggle-off disappeared with it and the window was left showing an anonymous avatar on black that nobody in the meeting could close. - Render the shared-video trigger's positioned wrapper only when the trigger itself renders, so a deployment with the feature off no longer lays an empty box over the corner of a tile that is itself the click target for pinning. * fix(multi-screen): bound the wait on the window-management permission An open that has to ask for the permission could stay pending forever. A call to getScreenDetails() does not settle while its prompt is on screen, and a prompt that is never answered never settles it at all, so the open produced no window, no error and no event: the embedder is promised an outcome for every setSecondScreen and got silence. The id also stayed in the in-flight set, so it could never be opened again for the life of the page. The prompt is only ever raised where the call carries transient user activation, which is the in-app triggers. Without it Chromium rejects immediately with NotAllowedError, which is why the external API path fails fast rather than hanging, and why headless does too (verified on 151: the API is present, the permission reports `prompt`, and the call rejects in ~2ms). The request is now bounded, and the timeout rejects into the existing failure path, so it is reported as window-management-unavailable exactly like a denial. The caller cannot tell the two apart, which is correct: either way the window cannot be placed on another screen. * fix(multi-screen): address the second review on the in-app triggers - Close an in-app second screen showing the whiteboard when the whiteboard is closed. Its source names no participant, so it is asked for by role like the shared video; it also needed a signature of its own, since a whiteboard resolves to neither a track nor a participant and signed as the constant `avatar:::`, which never changed and so never re-ran the check. - Stop deriving the window id from a screen index. Occupancy is positional now, so an id that also encodes a screen is a second answer to the same question that goes stale as soon as a display is undocked: a "that screen is free" decision could hand back the id of a window that already exists and re-source it, leaving the screen empty. Ids are now minted unique. - Only take over a window that is actually on an external screen. One the browser relocated onto the meeting's own display was still the oldest candidate, so a send could land on top of the meeting while the external screens kept what they had. - Treat a window whose position cannot be read as occupying its screen rather than as absent, which is what its docstring already promised. - Place the window off the permission answer instead of after the page load. moveTo/resizeTo need no document, and only the load listener has to be attached before the load event fires. A profile that has already denied the permission now closes the window at once rather than after a whole load, and a send cancelled while the prompt is up no longer flies to the other screen a moment before it is closed. * fix(multi-screen): register the second-screen window at load, not at permission Every finding in the last two review rounds lived in the gap between the window being open and its handle being in state, because the open held both the entry and the in-flight guard while it waited on the window-management permission. A cancel in that gap found no handle for closeSecondScreenHandle to close, so it reported nothing and left the trigger inverted for up to the full bound, where master reports secondScreenClosed promptly. A repeat send returned silently at the guard instead of re-sourcing. The handle is now registered as soon as the shell page loads, which is the point at which the window can actually be rendered into and closed. Placement becomes a tail hanging off the permission answer, holding no guard, so: - a cancel, a source leaving the meeting and a conference end all take the ordinary close path and report secondScreenClosed - a repeat send finds a live handle and re-sources it - the guard now spans the page load and nothing else The tail re-checks ownership by handle identity before it touches anything, so a window closed and reopened for the same id is neither placed nor torn down by the open it replaced. A denial or an unanswered prompt still fails the open, closing the window and reporting window-management-unavailable. The trade is deliberate: the portal is now built before the permission is known, so a refusal tears down a live window rather than an empty one, and the window renders on the meeting's own screen while the prompt is up rather than sitting blank. Fullscreen moves into the tail behind placement, or the window would fill the meeting's own screen for as long as the prompt is up. * fix(multi-screen): report a refused permission as an error alone - failSecondScreenOpen now detaches the handle before it removes the entry, so the middleware's REMOVE handler finds none and the removal stays silent - a denied window-management permission therefore reports secondScreenError on its own, as master does, instead of following it with a secondScreenClosed that an embedder reopening on that event would loop on - the denial is the only failure that reaches there with a handle in state, since it is the one that now comes after registration, but the detach covers any later failure the same way - detaching first also unmounts the portal while the window is still open, which is the ordering the REMOVE handler documents - record why applySource stays at registration: the subscriber calls it for every entry with a handle, so deferring the first event behind placement would only make it intermittent, and would lose it where placement throws on a live window * fix(multi-screen): give a departed source a grace period - a source that names a participant is now given 5s to come back before its window is closed, since a presence flap or a client rejoining is indistinguishable from a departure at the moment the check runs - closing is one-way, because the trigger left with the tile, so a false positive costs a window the user cannot get back, while being slow costs a few seconds of a window already showing an avatar - the countdown is stamped with the send it belongs to, so a window closed and reopened, or re-sourced, under a reused in-app id is not torn down by the countdown of the send it replaced - it is not restarted on every refresh, or another window's traffic would hold the deadline open indefinitely, and it is cancelled when the source comes back or the conference ends - the whiteboard and the shared video still close at once: those only stop when someone stops them, so a delay there would just lag a deliberate action
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )