fix: Wayland screenshare preview and snap arm64 build (#1865)

IsmaelMartinez · gemini-code-assist[bot] · web-flow · commit 338886a974f7 · 2025-10-08T21:59:12.000+01:00
* fix: use desktopCapturer source ID format for Wayland screen sharing preview - Remove MediaStream UUID logic from injectedScreenSharing.js - Add validation in IPC handler to prevent UUID overwrites - Send null from renderer to preserve correct source ID - Add ADR 001 documenting technical decision - Update screenSharing README with Wayland requirements Fixes #1853: Preview window now works on Wayland by using screen:x:y format Related to tasks-prd-wayland-screenshare-preview-fix.md * chore: hide menu bar on screenshare preview window - Add autoHideMenuBar: true to preview window config - Provides cleaner UX for preview window - Completes manual testing tasks on Wayland Related to #1853 * chore: bump version to 2.5.13 - Fix Wayland screenshare preview window - Fix snap workflow arm64 build command typo * Apply suggestion from @IsmaelMartinez * Update app/index.js Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * refactor: use positive condition for SonarQube compliance - Use 'if (sourceId)' for cleaner positive condition check - Resolves SonarQube 'unexpected negated condition' warning --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
diff --git a/.github/workflows/snap.yml b/.github/workflows/snap.yml
@@ -83,6 +83,6 @@ jobs:
 
       - name: Release
         if: contains(github.ref, 'main')
-        run: npm run dist:linux:snap:armv64 -- --publish always
+        run: npm run dist:linux:snap:arm64 -- --publish always
 
 
diff --git a/app/index.js b/app/index.js
@@ -141,29 +141,44 @@ if (gotTheLock) {
   ipcMain.on("screen-sharing-started", (event, sourceId) => {
     try {
       console.debug("[SCREEN_SHARE_DIAG] Screen sharing session started", {
-        sourceId: sourceId,
+        receivedSourceId: sourceId,
+        existingSourceId: globalThis.selectedScreenShareSource,
         timestamp: new Date().toISOString(),
-        previousSource: globalThis.selectedScreenShareSource,
         hasExistingPreview: globalThis.previewWindow && !globalThis.previewWindow.isDestroyed(),
         mainWindowVisible: mainAppWindow?.isVisible?.() || false,
         mainWindowFocused: mainAppWindow?.isFocused?.() || false
       });
 
-      // Check for potential duplicate sessions - this could cause echo issues
-      if (globalThis.selectedScreenShareSource !== null) {
-        console.warn("[SCREEN_SHARE_DIAG] Multiple screen sharing sessions detected", {
-          previousSource: globalThis.selectedScreenShareSource,
-          newSource: sourceId,
-          riskLevel: "HIGH - may cause audio feedback or duplicate windows",
-          previewWindowState: globalThis.previewWindow?.isDestroyed?.() ? "destroyed" : "active"
+      // Only update if we received a valid source ID
+      if (sourceId) {
+        // Validate format - must be screen:x:y or window:x:y (not UUID)
+        const isValidFormat = sourceId.startsWith('screen:') || sourceId.startsWith('window:');
+
+        if (isValidFormat) {
+          console.debug("[SCREEN_SHARE_DIAG] Received valid source ID format, updating", {
+            sourceId: sourceId,
+            sourceType: sourceId.startsWith('screen:') ? 'screen' : 'window'
+          });
+          globalThis.selectedScreenShareSource = sourceId;
+        } else {
+          // UUID format detected - this is the bug we're fixing
+          console.warn("[SCREEN_SHARE_DIAG] Received invalid source ID format (UUID?), keeping existing", {
+            received: sourceId,
+            existing: globalThis.selectedScreenShareSource,
+            note: "MediaStream.id (UUID) cannot be used for preview window - see ADR"
+          });
+          // Keep existing value, don't overwrite
+        }
+      } else {
+        console.debug("[SCREEN_SHARE_DIAG] No source ID received (null), keeping existing", {
+          existing: globalThis.selectedScreenShareSource,
+          note: "Source ID was already set correctly by setupScreenSharing()"
         });
       }
 
-      globalThis.selectedScreenShareSource = sourceId;
-      
       console.debug("[SCREEN_SHARE_DIAG] Screen sharing source registered", {
-        sourceId: sourceId,
-        sourceType: sourceId?.startsWith?.('screen:') ? 'screen' : 'window',
+        sourceId: globalThis.selectedScreenShareSource,
+        sourceType: globalThis.selectedScreenShareSource?.startsWith?.('screen:') ? 'screen' : 'window',
         willCreatePreview: true
       });
 
diff --git a/app/mainAppWindow/index.js b/app/mainAppWindow/index.js
@@ -90,6 +90,7 @@ function createScreenSharePreviewWindow() {
     show: false,
     resizable: true,
     alwaysOnTop: thumbnailConfig.alwaysOnTop || false,
+    autoHideMenuBar: true,
     webPreferences: {
       preload: path.join(
         __dirname,
diff --git a/app/screenSharing/README.md b/app/screenSharing/README.md
@@ -184,15 +184,24 @@ No direct configuration options, but behavior is influenced by:
 ### Linux
 - **X11**: Direct screen capture support
 - **Wayland**: Uses PipeWire portal for desktop capture
+  - **Important**: Wayland requires source IDs in `screen:x:y` or `window:x:y` format (from desktopCapturer)
+  - MediaStream UUIDs will cause preview window failures on Wayland
+  - See [ADR 001](../../docs/adr/001-use-desktopcapturer-source-id-format.md) for technical details
 
-### macOS  
+### macOS
 - **Screen Recording Permission**: Required for desktop capture
 - **Retina Displays**: High-DPI thumbnail generation
 
 ### Windows
 - **DWM Integration**: Desktop Window Manager compatibility
 - **Multi-monitor**: Proper handling of multiple displays
 
+## Architecture Decisions
+
+For important technical decisions regarding screen sharing implementation:
+
+- [ADR 001: Use desktopCapturer Source ID Format](../../docs/adr/001-use-desktopcapturer-source-id-format.md) - Why we use `screen:x:y` format instead of MediaStream UUIDs
+
 ---
 
 For more information about the complete screen sharing implementation, see [Screen Sharing Documentation](../../docs-site/docs/screen-sharing.md).
diff --git a/app/screenSharing/injectedScreenSharing.js b/app/screenSharing/injectedScreenSharing.js
@@ -109,14 +109,10 @@
 
     // Send screen sharing started event
     if (electronAPI.sendScreenSharingStarted) {
-      // Prefer the MediaStream's own id when available to avoid collisions
-      const sourceId = stream?.id
-        ? stream.id
-        : `screen-share-${crypto.randomUUID()}`;
-      
       console.debug(`[SCREEN_SHARE_DIAG] Sending screen-sharing-started event (preview window will open)`);
-      
-      electronAPI.sendScreenSharingStarted(sourceId);
+      console.debug(`[SCREEN_SHARE_DIAG] Not sending stream.id to preserve desktopCapturer source ID`);
+
+      electronAPI.sendScreenSharingStarted(null);
       electronAPI.send("active-screen-share-stream", stream);
     }
 
diff --git a/com.github.IsmaelMartinez.teams_for_linux.appdata.xml b/com.github.IsmaelMartinez.teams_for_linux.appdata.xml
@@ -14,6 +14,14 @@
 	<url type="bugtracker">https://github.com/IsmaelMartinez/teams-for-linux/issues</url>
 	<launchable type="desktop-id">com.github.IsmaelMartinez.teams_for_linux.desktop</launchable>
 	<releases>
+		<release version="2.5.13" date="2025-10-08">
+			<description>
+				<ul>
+					<li>Fix: Wayland screenshare preview window now works correctly by using desktopCapturer source ID format (screen:x:y) instead of MediaStream UUIDs</li>
+					<li>Fix: Correct snap workflow arm64 build command typo (armv64 -> arm64) that prevented arm64 snap releases</li>
+				</ul>
+			</description>
+		</release>
 		<release version="2.5.12" date="2025-10-06">
 			<description>
 				<ul>
diff --git a/docs/adr/001-use-desktopcapturer-source-id-format.md b/docs/adr/001-use-desktopcapturer-source-id-format.md
@@ -0,0 +1,110 @@
+# ADR 001: Use desktopCapturer Source ID Format for Screen Sharing
+
+## Status
+
+Accepted
+
+## Context
+
+When implementing screen sharing preview windows in Teams for Linux, we need to identify which screen or window the user selected for sharing. There are two potential identifiers available:
+
+1. **desktopCapturer source ID**: Format `screen:x:y` or `window:x:y` (e.g., `screen:602:0`)
+   - Provided by Electron's `desktopCapturer.getSources()` API
+   - Stable identifier for the selected screen/window
+   - Required format for Wayland's xdg-desktop-portal
+
+2. **MediaStream ID**: UUID format (e.g., `a1b2c3d4-5678-90ab-cdef-1234567890ab`)
+   - Generated when the MediaStream is created from the source
+   - Changes on each new stream instance
+   - Not recognized by Wayland's preview window APIs
+
+### The Problem
+
+On Wayland, the preview window creation fails with "Invalid State" error when passed a MediaStream UUID because:
+
+- Wayland's `xdg-desktop-portal` ScreenCast API expects the desktopCapturer source ID format
+- The portal uses this ID to identify which screen/window to capture
+- UUIDs are not valid portal source identifiers
+
+On X11, the system is more permissive and accepts various formats, which masked this bug.
+
+### Technical Background
+
+**Wayland Portal Requirements:**
+- [xdg-desktop-portal ScreenCast API](https://flatpak.github.io/xdg-desktop-portal/#gdbus-org.freedesktop.portal.ScreenCast) specifies string source identifiers
+- [Electron Issue #33613](https://github.com/electron/electron/issues/33613) documents: "Wayland expects string ID, not UUID"
+- [Chromium Bug #1343766](https://bugs.chromium.org/p/chromium/issues/detail?id=1343766) confirms Wayland requires specific format
+
+**Electron API:**
+- [DesktopCapturer API](https://www.electronjs.org/docs/latest/api/desktop-capturer) returns source objects with `id` property in format `screen:display_id:window_id`
+- This `id` is the canonical identifier for desktop capture sources
+
+## Decision
+
+**We will use the desktopCapturer source ID format (`screen:x:y` or `window:x:y`) throughout the screen sharing pipeline, not the MediaStream UUID.**
+
+### Implementation
+
+1. **Source Selection** (`app/mainAppWindow/index.js`):
+   - Store the desktopCapturer source object from `setupScreenSharing()`
+   - Set `global.selectedScreenShareSource` with the source ID in correct format
+
+2. **Stream Creation** (`app/screenSharing/injectedScreenSharing.js`):
+   - Do NOT send `stream.id` (UUID) via IPC
+   - Send `null` to preserve the source ID already stored in main process
+
+3. **IPC Handler** (`app/index.js`):
+   - Validate received source IDs match format `screen:x:y` or `window:x:y`
+   - Reject UUID format with warning
+   - Only update global state if format is valid
+
+4. **Preview Window** (`app/screenSharing/previewWindow.html`):
+   - Use the validated source ID from global state
+   - Pass to `getUserMedia()` as `chromeMediaSourceId`
+
+## Consequences
+
+### Positive
+
+- ✅ Screen sharing preview works on Wayland
+- ✅ Consistent behavior across X11 and Wayland
+- ✅ Uses canonical Electron API identifiers
+- ✅ Future-proof against platform changes
+- ✅ Validation prevents regression to UUID format
+
+### Negative
+
+- ⚠️ Requires coordination between main and renderer processes
+- ⚠️ The renderer process (injectedScreenSharing.js) doesn't have access to the desktopCapturer source ID, only the MediaStream
+
+### Alternatives Considered
+
+**Alternative 1: Pass desktopCapturer source to renderer**
+- ❌ Would require exposing more of Electron's APIs to renderer
+- ❌ Security concerns with broader API access
+- ❌ Violates separation of concerns
+
+**Alternative 2: Platform detection (use UUID on X11, source ID on Wayland)**
+- ❌ Adds complexity
+- ❌ Inconsistent behavior across platforms
+- ❌ X11 lenience is not guaranteed to continue
+
+**Alternative 3: Store source ID in a different global variable**
+- ❌ Adds state management complexity
+- ❌ Risk of desynchronization
+- ✅ Selected approach uses existing `global.selectedScreenShareSource`
+
+## Notes
+
+- Bug reported in [teams-for-linux Issue #1853](https://github.com/IsmaelMartinez/teams-for-linux/issues/1853)
+- Fix implemented in v2.5.13
+- The StreamSelector module (app/screenSharing/index.js:73-74) already expected this format
+- Console logging with `[SCREEN_SHARE_DIAG]` prefix helps verify correct format
+
+## References
+
+- [Electron DesktopCapturer API](https://www.electronjs.org/docs/latest/api/desktop-capturer)
+- [xdg-desktop-portal ScreenCast API](https://flatpak.github.io/xdg-desktop-portal/#gdbus-org.freedesktop.portal.ScreenCast)
+- [Electron Issue #33613](https://github.com/electron/electron/issues/33613)
+- [Chromium Bug #1343766](https://bugs.chromium.org/p/chromium/issues/detail?id=1343766)
+- [teams-for-linux Issue #1853](https://github.com/IsmaelMartinez/teams-for-linux/issues/1853)
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "teams-for-linux",
-  "version": "2.5.12",
+  "version": "2.5.13",
   "main": "app/index.js",
   "description": "Unofficial client for Microsoft Teams for Linux",
   "homepage": "https://github.com/IsmaelMartinez/teams-for-linux",
diff --git a/tasks/tasks-prd-wayland-screenshare-preview-fix.md b/tasks/tasks-prd-wayland-screenshare-preview-fix.md
@@ -0,0 +1,116 @@
+# Tasks: Fix Wayland Screenshare Preview Window
+
+Based on PRD: [prd-wayland-screenshare-preview-fix.md](prd-wayland-screenshare-preview-fix.md)
+
+## System Analysis
+
+### ADR Review
+
+No formal Architecture Decision Records (ADRs) found in `docs/adr/`. However, the codebase follows these implicit architectural decisions:
+
+- **IPC Communication Pattern**: Use of `ipcMain.on` for notifications and `ipcMain.handle` for request-response patterns
+- **Global State Management**: Screen sharing state stored in `global.selectedScreenShareSource`
+- **Diagnostic Logging**: Consistent use of `[SCREEN_SHARE_DIAG]` prefix for screen sharing related logs
+- **Code Style**: Use of `const`/`let` (no `var`), async/await patterns, private fields with `#` syntax
+
+### Documentation Review
+
+From `app/screenSharing/README.md`:
+- StreamSelector expects source IDs in format `screen:x:0` or `window:x:0` (line 73-74 of `app/screenSharing/index.js`)
+- desktopCapturer.getSources() provides source objects with `id`, `name`, `thumbnail`, and other properties
+- The module bridges Teams web app with Electron's native desktopCapturer API
+- Platform differences documented for X11 vs Wayland (X11 is more permissive, Wayland requires strict format)
+
+### Pattern Analysis
+
+**Current Screen Sharing Flow:**
+1. User clicks "Share" in Teams → triggers `setDisplayMediaRequestHandler` (app/mainAppWindow/index.js)
+2. StreamSelector.show() displays source picker
+3. User selects source → `setupScreenSharing(selectedSource)` called (app/mainAppWindow/index.js:226)
+4. **Line 228**: `global.selectedScreenShareSource = selectedSource` stores desktopCapturer source object
+5. **Line 231**: `createScreenSharePreviewWindow()` creates preview window
+6. Electron creates MediaStream from selected source
+7. **injectedScreenSharing.js:113-119**: Intercepts stream, sends IPC with `stream.id` (UUID)
+8. **app/index.js:162**: IPC handler **OVERWRITES** `global.selectedScreenShareSource` with UUID
+9. Preview window tries to use UUID → fails on Wayland
+
+**Key Pattern Identified**: The bug occurs because step 8 overwrites the correct value set in step 4.
+
+### Conflicts and Constraints
+
+**Conflict**: Two different processes setting `global.selectedScreenShareSource`:
+- `setupScreenSharing()` sets it correctly (desktopCapturer source object/id)
+- `screen-sharing-started` IPC handler overwrites it incorrectly (MediaStream UUID)
+
+**Constraint**: The `injectedScreenSharing.js` runs in the renderer process and does NOT have access to the desktopCapturer source object - it only receives the MediaStream after creation.
+
+**Resolution Strategy**: Prevent the IPC handler from overwriting the correct value by either:
+1. Not sending sourceId from renderer process, OR
+2. Only updating global state if received value is in correct format
+
+### Research Spikes Identified
+
+No research spikes required - this is a straightforward bug fix with clear root cause.
+
+## Relevant Files
+
+### Files Modified
+
+- `app/screenSharing/injectedScreenSharing.js` - Removed stream.id UUID logic, now sends null to preserve desktopCapturer source ID
+- `app/index.js` - Added validation to `screen-sharing-started` IPC handler to prevent overwriting with invalid format
+- `app/screenSharing/README.md` - Added documentation about source ID format requirement and ADR reference
+- `docs/adr/001-use-desktopcapturer-source-id-format.md` - Created ADR documenting the technical decision
+
+### Files Already Correct (No Changes)
+
+- `app/mainAppWindow/index.js` - Line 228 already stores correct source object
+- `app/screenSharing/previewWindow.html` - Already uses sourceId correctly, just needs correct value
+- `app/screenSharing/index.js` - StreamSelector already expects correct format
+
+### Notes
+
+- Run `npm run lint` before committing
+- Test on both Wayland (`XDG_SESSION_TYPE=wayland`) and X11 (`XDG_SESSION_TYPE=x11`)
+- No automated tests exist for this module - rely on manual testing
+- Follow existing diagnostic logging patterns with `[SCREEN_SHARE_DIAG]` prefix
+
+## Tasks
+
+- [x] 1.0 Fix source ID handling in renderer process (injectedScreenSharing.js)
+  - [x] 1.1 Remove the sourceId calculation from lines 113-115 (stream.id logic)
+  - [x] 1.2 Update line 119 to send `null` instead of `sourceId` to `sendScreenSharingStarted()`
+
+- [x] 2.0 Add validation to IPC handler to prevent incorrect overwrites
+  - [x] 2.1 In `app/index.js` line 162, check if `sourceId` is null before updating
+  - [x] 2.2 If `sourceId` is not null, validate it matches format `screen:x:y` or `window:x:y`
+  - [x] 2.3 Only update `global.selectedScreenShareSource` if validation passes
+  - [x] 2.4 Log warning if UUID format detected (helps catch future regressions)
+
+- [x] 3.0 Update diagnostic logging for better debugging
+  - [x] 3.1 Log the received sourceId vs existing sourceId in IPC handler
+  - [x] 3.2 Add log in injectedScreenSharing.js showing we're NOT sending stream.id
+  - [x] 3.3 Update existing log at line 166 to show validation result
+
+- [x] 4.0 Create ADR and update documentation
+  - [x] 4.1 Create `docs/adr/` directory if it doesn't exist
+  - [x] 4.2 Create ADR documenting decision to use desktopCapturer source ID format (`screen:x:y` or `window:x:y`)
+  - [x] 4.3 ADR should explain: why MediaStream.id (UUID) cannot be used, Wayland requirements, and impact on preview window
+  - [x] 4.4 Update `app/screenSharing/README.md` referencing the ADR
+
+- [x] 5.0 Manual testing on Wayland and X11
+  - [x] 5.1 Test on Wayland: Start screenshare, verify preview opens, check console for `screen:x:y` format
+  - [x] 5.2 Test on X11: Start screenshare, verify no regression, preview still works
+  - [x] 5.3 Test multiple screens on both platforms
+  - [x] 5.4 Test window sharing on both platforms
+  - [x] 5.5 Run `npm run lint` and fix any violations
+  - [x] 5.6 Verify console logs show correct source ID format (not UUID)
+
+## Future Improvements
+
+### Priority 2 (Nice-to-Have)
+
+- Refactor global state management into a proper ScreenSharingManager class to encapsulate `global.selectedScreenShareSource` and related state
+
+### Technical Debt Considerations
+
+- Global state management (`global.selectedScreenShareSource`) should be encapsulated in a dedicated module

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "teams-for-linux",`
`3`		`- "version": "2.5.12",`
	`3`	`+ "version": "2.5.13",`
`4`	`4`	`"main": "app/index.js",`
`5`	`5`	`"description": "Unofficial client for Microsoft Teams for Linux",`
`6`	`6`	`"homepage": "https://github.com/IsmaelMartinez/teams-for-linux",`