Skip to content

Conversation

@yaira2
Copy link
Member

@yaira2 yaira2 commented Jun 19, 2024

Resolved / Related Issues

To prevent extra work, all changes to the Files codebase must link to an approved issue marked as Ready to build. Please insert the issue number following the hashtag with the issue number that this Pull Request resolves.

Steps used to test these changes

Stability is a top priority for Files and all changes are required to go through testing before being merged into the repo. Please include a list of steps that you used to test this PR.

  1. Followed the repro steps outlined in issue Bug: Shortcut set to run as Admin doesn't work if there are arguments #15595.
  2. Verified that arguments are correctly passed in both scenarios: when the shortcut is configured to run as an administrator and when it is not.

@yaira2 yaira2 added the ready for review Pull requests that are ready for review label Jun 19, 2024
@yaira2
Copy link
Member Author

yaira2 commented Jun 19, 2024

In the long run, we should modify HandleApplicationLaunch to handle this correctly. It the meantime, I think this is a good workaround to avoid a refactor.

@yaira2 yaira2 requested a review from gave92 June 19, 2024 21:57
@0x5bfa
Copy link
Member

0x5bfa commented Jun 20, 2024

In the long run, we should modify HandleApplicationLaunch to handle this correctly. It the meantime, I think this is a good workaround to avoid a refactor.

Could you add a NOTE comment saying this, so I (or someone) can notice?

@gave92
Copy link
Member

gave92 commented Jun 22, 2024

I'd probably go with the refactor route, it's not that bad, see here:
https://github.com/gave92/files-uwp/tree/rev_adminargs

If you prefer the current solution, make sure to remove "process.WaitForExit();" and to wrap in a try-catch in case user says no to UAC, like:
cfb1af0

@yaira2
Copy link
Member Author

yaira2 commented Jun 23, 2024

I'd probably go with the refactor route, it's not that bad, see here: https://github.com/gave92/files-uwp/tree/rev_adminargs

If you prefer the current solution, make sure to remove "process.WaitForExit();" and to wrap in a try-catch in case user says no to UAC, like: cfb1af0

Your way is fine with me, can you open a PR?

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed ready for review Pull requests that are ready for review labels Jun 24, 2024
@yaira2 yaira2 merged commit 0d7d2aa into main Jun 24, 2024
@yaira2 yaira2 deleted the ya/RunArgsAdmin branch June 24, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to merge Pull requests that are approved and ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Shortcut set to run as Admin doesn't work if there are arguments

4 participants