Ready for Review - [Mouse Without Borders] - refactoring "Common" classes (Part 4) - #35155 #37579

mikeclayton · 2025-02-22T17:55:47Z

Summary of the Pull Request

Part 4 of a slow-running refactor of the giant "Common" class in Mouse Without Borders into individual classes with tighter private scope.

Extract the "Common" code from the following files:
- Common.Event.cs -> Event.cs
- Common.Helper.cs -> Helper.cs
- Common.Launch.cs -> Launch.cs
- Common.Service.cs -> Service.cs
Update references to the types in the new locations
Update unit test to verify functionality has only changed in an expected way

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Run manual tests from Test Checklist Template:

Install PowerToys on two PCs in the same local network:
- Verify that PowerToys is properly installed on both PCs.
Setup Connection:
- Open MWB's settings on the first PC and click the "New Key" button. Verify that a new security key is generated.
- Copy the generated security key and paste it in the corresponding input field in the settings of MWB on the second PC. Also enter the name of the first PC in the required field.
- Press "Connect" and verify that the machine layout now includes two PC tiles, each displaying their respective PC names.
Verify Connection Status:
- Ensure that the border of the remote PC turns green, indicating a successful connection.
- Enter an incorrect security key and verify that the border of the remote PC turns red, indicating a failed connection.
Test Remote Mouse/Keyboard Control:
- With the PCs connected, test the mouse/keyboard control from one PC to another. Verify that the mouse/keyboard inputs are correctly registered on the other PC.
- Test remote mouse/keyboard control across all four PCs, if available. Verify that inputs are correctly registered on each connected PC when the mouse is active there.
  - unable to test - only 2 machines available
Test Remote Control with Elevated Apps:
- note - the main PowerToys.exe must be running as a non-admin for these tests
- Open an elevated app on one of the PCs. Verify that without "Use Service" enabled, PowerToys does not control the elevated app.
- Enable "Use Service" in MWB's settings (need to run PowerToys.exe as admin to enable "Use Service", then restart PowerToys.exe as non-admin). Verify that PowerToys can now control the elevated app remotely. Verify that MWB processes are running as LocalSystem, while the MWB helper process is running non-elevated.
  - get-process -Name "PowerToys.MouseWithoutBorders*" -IncludeUserName | format-table Id, ProcessName, UserName
  - Process: PowerToys.MouseWithoutBorders.exe - running as SYSTEM
  - Process: PowerToys.MouseWithoutBorders.Helper.exe - running as current user
  - get-service -Name "PowerToys.*" | ft Status, Name, UserName; get-ciminstance -Class "Win32_Service" -Filter "Name like 'PowerToys%'" | ft ProcessId, Name
  - Service: PowerToys.MWB.Service - running as Local System
- Toggle "Use Service" again, verify that each time you do that, the MWB processes are restarted.
- Run PowerToys elevated on one of the machines, verify that you can control elevated apps remotely now on that machine.
Test Module Enable Status:
- For all combinations of "Use Service"/"Run PowerToys as admin", try enabling/disabling MWB module and verify that it's indeed being toggled using task manager.
Test Disconnection/Reconnection:
- Disconnect one of the PCs from network. Verify that the machine layout updates to reflect the disconnection.
- Do the same, but now by exiting PowerToys.
- Start PowerToys again, verify that the PCs are reconnected.
Test Various Local Network Conditions:
- Test MWB performance under various network conditions (e.g., low bandwidth, high latency). Verify that the tool maintains a stable connection and functions correctly.
Clipboard Sharing:
- Copy some text on one PC and verify that the same text can be pasted on another PC.
- Use the screenshot key and Win+Shift+S to take a screenshot on one PC and verify that the screenshot can be pasted on another PC.
- Copy a file in Windows Explorer and verify that the file can be pasted on another PC. Make sure the file size is below 100MB.
- Try to copy multiple files and directories and verify that it's not possible (only the first selected file is being copied).
Drag and Drop:
- Drag a file from Windows Explorer on one PC, cross the screen border onto another PC, and release it there. Verify that the file is copied to the other PC. Make sure the file size is below 100MB.
- While dragging the file, verify that a corresponding icon is displayed under the mouse cursor.
- Without moving the mouse from one PC to the target PC, press CTRL+ALT+F1/2/3/4 hotkey to switch to the target PC directly and verify that file sharing/dropping is not working.
Lock and Unlock with "Use Service" Enabled:
- Enable "Use Service" in MWB's settings.
- Lock a remote PC using Win+L, move the mouse to it remotely, and try to unlock it. Verify that you can unlock the remote PC.
- Disable "Use Service" in MWB's settings, lock the remote PC, move the mouse to it remotely, and try to unlock it. Verify that you can't unlock the remote PC.
Test Settings:
- Change the rest of available settings on MWB page and verify that each setting works as described.

Group Policy Tests

See https://learn.microsoft.com/en-us/windows/powertoys/grouppolicy

…rosoft#35155

…icrosoft#35155

Copilot

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Copilot · 2025-02-25T02:17:04Z

src/modules/MouseWithoutBorders/App/Core/Service.cs

+    internal static void StartMouseWithoutBordersService(string desktopToRunMouseWithoutBordersOn = null, string startTag1 = "byapp", string startTag2 = null)
+    {
+        // NOTE(@yuyoyuppe): the new flow assumes we run both mwb processes directly from the svc.
+        if (Common.RunWithNoAdminRight || true)


The use of '|| true' in the conditional always evaluates to true, bypassing the intended service start logic. Consider removing '|| true' or revising the condition to properly control service startup.

Suggested change

if (Common.RunWithNoAdminRight || true)

if (Common.RunWithNoAdminRight)

if (Common.RunWithNoAdminRight || true) was in the original Common.Service.cs file - Copilot's detected it as it thinks Service.cs is a new file rather than a rename and is treating this as a "new" issue. I'd suggest leaving as-is for this PR.

jaimecbernardo · 2025-02-27T11:28:29Z

Hi @mikeclayton ,
Thank you for opening this PR.
Is this ready for review yet? Looks like reviews are already being asked?

mikeclayton · 2025-02-27T12:38:06Z

@jaimecbernardo - I didn't request the review as I've not properly worked through the tests yet, but it compiles locally and superficially appears to work.

Happy to follow your lead though - I don't mind if you want to wait until I've finished testing before starting the review process. I already pushed one set of changes (#36950) into the next release (v0.89.0), so might be better to defer this PR until the release after to avoid too much churn?

mikeclayton · 2025-03-06T09:51:25Z

@jaimecbernardo - I've worked through most of the tests and it all seems to work, so I think this PR is ready for review now...

mikeclayton added 6 commits February 21, 2025 18:50

[MouseWithoutBorders] - moving Common.Event.cs -> Core\Event.cs - mic…

c31e671

…rosoft#35155

[MouseWithoutBorders] - moving Common.Service.cs -> Core\Service.cs - m…

4885c30

…icrosoft#35155

[MouseWithoutBorders] - moving Common.Launch.cs -> Core\Launch.cs - m…

e4288d0

…icrosoft#35155

[MouseWithoutBorders] - moving Common.Helper.cs -> Core\Helper.cs - m…

f208794

…icrosoft#35155

[MouseWithoutBorders] - refactoring and fixes for logger unit test - m…

dd7c7d2

…icrosoft#35155

[MouseWithoutBorders] - cleaning up changes - microsoft#35155

1077d84

mikeclayton mentioned this pull request Feb 22, 2025

[Mouse Without Borders] - refactoring "Common" classes #35155

Open

yeelam-gordon requested review from vanzue and Copilot February 25, 2025 02:15

Copilot AI reviewed Feb 25, 2025

View reviewed changes

yeelam-gordon requested a review from drawbyperpetual February 25, 2025 02:17

mikeclayton marked this pull request as ready for review March 6, 2025 09:51

mikeclayton changed the title ~~WIP - [Mouse Without Borders] - refactoring "Common" classes (Part 4) - #35155~~ Ready for Review - [Mouse Without Borders] - refactoring "Common" classes (Part 4) - #35155 Mar 6, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ready for Review - [Mouse Without Borders] - refactoring "Common" classes (Part 4) - #35155 #37579

Ready for Review - [Mouse Without Borders] - refactoring "Common" classes (Part 4) - #35155 #37579

mikeclayton commented Feb 22, 2025 •

edited

Loading

Copilot AI left a comment

Copilot AI Feb 25, 2025

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

mikeclayton Mar 5, 2025

jaimecbernardo commented Feb 27, 2025

mikeclayton commented Feb 27, 2025 •

edited

Loading

mikeclayton commented Mar 6, 2025

	if (Common.RunWithNoAdminRight \|\| true)
	if (Common.RunWithNoAdminRight)

Ready for Review - [Mouse Without Borders] - refactoring "Common" classes (Part 4) - #35155 #37579

Are you sure you want to change the base?

Ready for Review - [Mouse Without Borders] - refactoring "Common" classes (Part 4) - #35155 #37579

Conversation

mikeclayton commented Feb 22, 2025 • edited Loading

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Run manual tests from Test Checklist Template:

Group Policy Tests

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot AI Feb 25, 2025

Choose a reason for hiding this comment

mikeclayton Mar 5, 2025

Choose a reason for hiding this comment

jaimecbernardo commented Feb 27, 2025

mikeclayton commented Feb 27, 2025 • edited Loading

mikeclayton commented Mar 6, 2025

mikeclayton commented Feb 22, 2025 •

edited

Loading

mikeclayton commented Feb 27, 2025 •

edited

Loading