Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

App menu shows "Sign in to sync" menu item until I tap it, even though I'm already signed in to sync #25486

Closed
cpeterso opened this issue Jun 1, 2022 · 9 comments · Fixed by #28409
Assignees
Labels
eng:qa:verified QA Verified Feature:MainMenu The three-dot menu that is seen on the browser and homescreen. needs:triage Issue needs triage Needs-UX Issues or tickets that need UX input or review
Milestone

Comments

@cpeterso
Copy link

cpeterso commented Jun 1, 2022

Steps to reproduce

  1. Log into your FxA Sync in Fenix.
  2. Kill Fenix.
  3. Launch Fenix.
  4. Open the app menu.
  5. See "Sign in to sync". I think this is a bug!
  6. Tap the "Settings" menu item.
  7. See at the top of the Settings screen that you are already signed in.
  8. Exit the Settings screen.
  9. Open the app menu again.
  10. See that the "Sign in to sync" menu item has been replaced with your sync account name. This is the result I expected in step 5 above.

Maybe I am experiencing #24112? But I don't think my account is in a bad authentication state.

┆Issue is synchronized with this Jira Task

@gabrielluong
Copy link
Member

We probably have multiple duplicate issues related to this #19657 and an existing PR that was approved but never landed #21488

@Alexandru2909
Copy link
Contributor

Alexandru2909 commented Aug 5, 2022

While investigating this I noticed that in the current implementation of HomeMenu, when the onAuthenticated callback is invoked, we update the menu builder, thus dismissing the menu, if it is being displayed at that moment. Updating the menu does not also update the syncItem, as it is stored as a class property.

This behavior is fixed in the contributor's PR, however the menu dismissing is something we may not want to add.

actual.mp4
contributor.mp4

Since the authentication state takes some time to be updated and notify the observers about it, the Sign in to sync item is displayed for a few seconds before the menu is dismissed.

There are several solutions we can apply to avoid this: One would be to avoid displaying the menu for those few seconds, another would be to add another state ("Authentication in progress..") until we get an update from the account manager.
Another solution we could use is not dismissing the already displayed menu, allowing the builder to be updated while also keeping the user's menu.

solution1.mp4

Tagging UX to check with which of these solution we want to go with, or keeping the current behavior.

@Alexandru2909 Alexandru2909 added Feature:MainMenu The three-dot menu that is seen on the browser and homescreen. Needs-UX Issues or tickets that need UX input or review labels Aug 5, 2022
@rocketsroger
Copy link
Contributor

Confirmed this has been fixed in Nightly. Closing

@ckarlof
Copy link

ckarlof commented Dec 19, 2022

I'm still experiencing this bug in Firefox Nightly 110.a1 (Build #201592213). The STR are the same as originally reported with the added step of disabling network before launching Fenix in step 3 (i.e., I experience this problem most frequently when I have poor network access).

@cpeterso
Copy link
Author

@rocketsroger This bug is still reproducible in Nightly 110.a1.

@cpeterso cpeterso reopened this Dec 20, 2022
@mavduevskiy mavduevskiy self-assigned this Jan 5, 2023
@mavduevskiy
Copy link
Contributor

Was able to find a quick fix, now need time to make sure it doesn't break anything. Implementations of HomeMenu and ToolbarMenu are quite different, and I have to make sure it works correctly in both cases.

The issue looks rather simple. The menu items are build upon the screen opening, but the account manager at that point doesn't have the actual state. It takes just a few millisecond for it to get updated usually, but we never update the items in the list. (with the exception of account error, but I have yet to verify that it works for authentication errors).

mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 5, 2023
@github-actions github-actions bot added the eng:reopen-for-qa Reopens and tags the issue for QA needed when the issue is merged label Jan 5, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 5, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 7, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 7, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 7, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 8, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 8, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 8, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 8, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 8, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 8, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 8, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 17, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 17, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 17, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 17, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 17, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 17, 2023
@mavduevskiy
Copy link
Contributor

The long term solution is caching the profile. A-S will provide us with the API to always get a cached profile (ticket), then we have to support it on the A-C and fenix side.

mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 18, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 18, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 18, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 19, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 19, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 19, 2023
mavduevskiy pushed a commit to mavduevskiy/fenix that referenced this issue Jan 20, 2023
@mergify mergify bot closed this as completed in #28409 Jan 20, 2023
mergify bot pushed a commit that referenced this issue Jan 20, 2023
@github-actions github-actions bot reopened this Jan 20, 2023
@github-actions github-actions bot added eng:qa:needed QA Needed and removed eng:reopen-for-qa Reopens and tags the issue for QA needed when the issue is merged labels Jan 20, 2023
t-p-white pushed a commit to t-p-white/fenix that referenced this issue Jan 20, 2023
@github-actions github-actions bot added the eng:reopen-for-qa Reopens and tags the issue for QA needed when the issue is merged label Jan 20, 2023
@github-actions github-actions bot reopened this Jan 25, 2023
@github-actions github-actions bot removed the eng:reopen-for-qa Reopens and tags the issue for QA needed when the issue is merged label Jan 25, 2023
@SoftVision-LorandJanos
Copy link

This issue is no longer reproducible with the latest Nightly 111.0a1 (2023-01-26) build.
The "Sign in to sync" menu item has been replaced with your sync account name on first time accessing the menu after logged in and closing-opening the app.
Device used: Oppo Find X5 (Android 12).
Closing the ticket as verified.

@SoftVision-LorandJanos SoftVision-LorandJanos added eng:qa:verified QA Verified and removed eng:qa:needed QA Needed labels Jan 26, 2023
akliuxingyuan pushed a commit to akliuxingyuan/iceraven-browser that referenced this issue Mar 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng:qa:verified QA Verified Feature:MainMenu The three-dot menu that is seen on the browser and homescreen. needs:triage Issue needs triage Needs-UX Issues or tickets that need UX input or review
Projects
None yet
9 participants