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

Revert "[Windows] FileSystem.Current.AppDataDirectory returns different path after updating to latest version - fix" #26685

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

mattleibow
Copy link
Member

Reverts #26039

I looked into this issue some more and I see we were always putting preferences and app data in the wrong location. They were always supposed to be stored in roaming as they are supposed to travel with the app.

All the way back in 8.0.70 and 9.0.0 preview 7 we "broke" the way we stored it: https://github.com/dotnet/maui/pull/23265/files#diff-68e97c448bfa67ad3c292a6e146185895b0c2c6437fe5f361d6044327b56dc84R39

However, we actually made it all consistent - we stored the values in local storage and not roaming storage.

Then in 9.0.21 we saw it and realised that we were wrong and reverted our bug: https://github.com/dotnet/maui/pull/26039/files

However, now it is more correct, but more inconsistent and breaking to our users.

Even though technically we will be incorrect to move preferences back to local storage, it will be the way we have always done things.

Fixes #26602

@Copilot Copilot bot review requested due to automatic review settings December 17, 2024 17:34
@mattleibow mattleibow requested a review from a team as a code owner December 17, 2024 17:34

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/Essentials/src/FileSystem/FileSystem.uwp.cs:39

  • Reverting to using Environment.SpecialFolder.LocalApplicationData instead of Environment.SpecialFolder.ApplicationData may cause data to not roam with the user, which can lead to data inconsistency across devices.
string path = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), AppSpecificPath, "Data");
@mattleibow
Copy link
Member Author

@MartyIX FYI

@PureWeen PureWeen added this to the .NET 9 SR2.2 milestone Dec 17, 2024
@jfversluis
Copy link
Member

@dotMorten

@jfversluis jfversluis added t/breaking 💥 area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info needs-breaking-change-doc-created labels Dec 18, 2024
@jfversluis jfversluis merged commit 3c4c27c into main Dec 18, 2024
104 checks passed
@jfversluis jfversluis deleted the revert-26039-feature/2024-11-21-windows-path branch December 18, 2024 09:06
@PureWeen
Copy link
Member

/backport to release/9.0.1xx-sr2

Copy link
Contributor

Started backporting to release/9.0.1xx-sr2: https://github.com/dotnet/maui/actions/runs/12397156824

@dotMorten
Copy link
Contributor

dotMorten commented Dec 18, 2024

They were always supposed to be stored in roaming as they are supposed to travel with the app.

This is quite dangerous and roaming should be an opt in. Not to mention if you use encryption linked to the device/account, it won't decrypt on the other machine, and will instead keep having to recreate and overwrite the encrypted data back and forth. Or store the path to last opened file wouldn't apply to the app. There's also size restrictions on roaming data, and most importantly it's no longer supported in Win11+
https://learn.microsoft.com/en-us/windows/apps/design/app-settings/store-and-retrieve-app-data#roaming-data

If anything you might need a way in the settings API to specify where you want it to go, so you can control it (which seem moot since the support is going away), but until that happens IMHO it should default to local.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info needs-breaking-change-doc-created t/breaking 💥
Projects
Status: Done
5 participants