search

mozilla-mobile / firefox-ios

Firefox for iOS
Mozilla Public License 2.0
12.16k stars 2.92k forks source link

[Intermittent][Addresses phase 2] All addresses are accidentally deleted from the user account #21940

Open alinamoldovan84 opened 3 weeks ago

alinamoldovan84 commented 3 weeks ago

Steps to reproduce

  1. Log into the app with the FF account
  2. Navigate to settings -> Addresses and make sure the list is populated with some addresses
  3. The user logs out from the FF app
  4. Navigate to settings -> Addresses and remove all the addresses
  5. Log into the app with the FF account
  6. Navigate to settings -> Addresses and check the addresses list

Expected behavior

All addresses are synced back on the device

Actual behavior

Sometimes all addresses are accidentally deleted from the user account. This issue is intermittent it happened 4 times out of 6

Device & build information

https://github.com/user-attachments/assets/33c0187a-f073-4a52-b760-0476b771a07b

image

┆Issue is synchronized with this Jira Task

data-sync-user commented 3 weeks ago

➤ Norberto Andres Furlan commented:

Issam Mani Razvan Litianu can you take a look to this one? I am worried about this one in particular.

data-sync-user commented 3 weeks ago

➤ Issam Mani commented:

I was able to reproduce this once and it does seem to be a sync issue since deletion is also propagated to Desktop. Will try to reproduce again and investigate.

data-sync-user commented 2 weeks ago

➤ Razvan Litianu commented:

Pretty hard to reproduce and follow the steps since it is intermitent but I was assuming it’s retaining cycle. I have a possible fix with the PR ( https://github.com/mozilla-mobile/firefox-ios/pull/22142 )

data-sync-user commented 2 weeks ago

➤ Issam Mani commented:

{quote}I have a possible fix with the PR ( https://github.com/mozilla-mobile/firefox-ios/pull/22142 ) {quote}

Thanks Razvan Litianu. I had a suspicion that somehow the logged in state for sync is retained somehow because deletion is propagated to desktop, but I could only reproduce this once 🤷‍♂️

data-sync-user commented 2 weeks ago

➤ Norberto Andres Furlan commented:

Is this one fixed? Issam Mani Razvan Litianu

data-sync-user commented 2 weeks ago

➤ Issam Mani commented:

Norberto Andres Furlan Razvan Litianu opened a possible fix. That’s our best bet right now once it’s merged. I will keep trying to reproduce this locally but so far I wasn’t successful.

data-sync-user commented 2 weeks ago

➤ Nishant Bhasin commented:

https://github.com/mozilla-mobile/firefox-ios/pull/22142 ( https://github.com/mozilla-mobile/firefox-ios/pull/22142|smart-link ) - This PR got merged

Issam Mani lets see how main does for this

data-sync-user commented 2 weeks ago

➤ Alina Moldovan commented:

This issue still reproduces intermittently using v131 (45674).

data-sync-user commented 2 weeks ago

➤ Issam Mani commented:

Update:

Made a bit of progress. It seems that we land in a broken state where we signed out of sync/fxa but the code still thinks it’s signed in. Notice the pref says true. This is the only case where the issue happens. I will investigate more to see what we do when we disconnect from sync.

!Group 1.png|width=1792,height=1039,alt="Group 1.png"!

data-sync-user commented 1 week ago

➤ Issam Mani commented:

Ok so update on this. Lougenia Bailey and I investigated this further yesterday ( slack thread ( https://mozilla.slack.com/archives/C0559DDDPQF/p1727363793404829 ) ) and two important findings came up:

The outcome of the discussion is that this is not a big problem to block release of address autofill, since it’s not address specific. It happens rarely and will only propagate the deletion of the addresses/bookmarks deleted when it happens not all of them. Also this is a pre-existing issue as we were able to reproduce this for bookmarks. We can fix this later.

Norberto Andres Furlan Nishant Bhasin what do you think ? This is now the only blocker for address autofill.