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

fix: Get the correct file path #1211

Merged
merged 1 commit into from
Aug 7, 2024
Merged

fix: Get the correct file path #1211

merged 1 commit into from
Aug 7, 2024

Conversation

eriklimakc
Copy link
Contributor

GitHub Issue (If applicable): #1165

PR Type

What kind of change does this PR introduce?

  • Bugfix

What is the current behavior?

The url being returned is file:\C:\C\1165\UnoApp1\UnoApp1\bin\Debug\net8.0-windows10.0.19041\AppxManifest.xml. When File.Exist uses that url, it returns false due to the "file" prefix.

What is the new behavior?

No "file" prefix.

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Internal Issue (If applicable):

@eriklimakc eriklimakc requested a review from kazo0 July 31, 2024 15:47
@eriklimakc eriklimakc self-assigned this Jul 31, 2024
@eriklimakc eriklimakc linked an issue Jul 31, 2024 that may be closed by this pull request
17 tasks
Copy link
Contributor

@Xiaoy312 Xiaoy312 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably wrap it in #If WINDOWS
given the problem is only affecting windows unpackaged
---
or, did you checked that this doesnt break/isnt reached on skia/wasm?

@kazo0
Copy link
Contributor

kazo0 commented Aug 5, 2024

@eriklimakc Can you double check the comment from @Xiaoy312 and check if either the change works fine for skia/wasm or if we need to wrap this in #if WINDOWS

@kazo0 kazo0 marked this pull request as draft August 5, 2024 13:39
@eriklimakc
Copy link
Contributor Author

we should probably wrap it in #If WINDOWS given the problem is only affecting windows unpackaged --- or, did you checked that this doesnt break/isnt reached on skia/wasm?

Skia: On skia the prefix 'file:' is also present on codebase and the code added works fine. It didn't change the behavior, since the file on the check below doesn't exist, so it loads from StorageFile.GetFileFromApplicationUriAsync(new Uri($"ms-appx:///{fileName}")).

var storageFile = await StorageFile.GetFileFromApplicationUriAsync(new Uri($"ms-appx:///{fileName}"));
filePath = storageFile.Path;

Wasm: I can't debug wasm, but I used typeof(ExtendedSplashScreen).Log()... to see if the code is reached, but I didn't see any of the logs I added on the Console. I didn't notice any change on the behavior.

I also downloaded the artifacts from this PR and used on a new uno app and the app didn't show any problems.

Are there other tests I could do to ensure this will run smoothly?

cc @Xiaoy312 @kazo0

@kazo0
Copy link
Contributor

kazo0 commented Aug 6, 2024

@eriklimakc

I would say we should just do what @Xiaoy312 mentioned and wrap this specific change in #If WINDOWS just so we can be sure we aren't changing the logic for other platforms

@eriklimakc eriklimakc marked this pull request as ready for review August 6, 2024 15:00
@kazo0
Copy link
Contributor

kazo0 commented Aug 7, 2024

@Mergifyio backport release/stable/6.1

Copy link
Contributor

mergify bot commented Aug 7, 2024

backport release/stable/6.1

✅ Backports have been created

@kazo0 kazo0 merged commit 340a673 into main Aug 7, 2024
25 checks passed
@kazo0 kazo0 deleted the dev/ERLI/1165-findappxfile branch August 7, 2024 13:37
mergify bot pushed a commit that referenced this pull request Aug 7, 2024
kazo0 pushed a commit that referenced this pull request Aug 7, 2024
Co-authored-by: Érik Lima <114886335+eriklimakc@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ExtendedSplashScreen throws exception running as unpackaged
3 participants