Closed Bug 1686521 Opened 2 years ago Closed 1 year ago

Show FxA button based on signed-in state

Categories

(Firefox :: Toolbars and Customization, enhancement, P1)

enhancement

Tracking

()

VERIFIED FIXED
88 Branch
Tracking Status
firefox88 --- verified
firefox89 --- verified

People

(Reporter: mstriemer, Assigned: sfoster)

References

(Blocks 1 open bug, Regressed 4 open bugs)

Details

(Whiteboard: [proton-toolbar])

Attachments

(1 file)

In the proton specs, the FxA button is to be hidden from the toolbar when the user is not signed in. Once the user signs in, the button should be added to the toolbar automatically, but should still be customizable.

Whiteboard: [proton-toolbar]
Assignee: nobody → sfoster
Status: NEW → ASSIGNED

Hey sfoster, do we know if this behaviour is expected to be behind a Proton pref, or is it able to ride out earlier?

Flags: needinfo?(sfoster)

(In reply to Mike Conley (:mconley) (:⚙️) (Catching up on needinfos) from comment #1)

Hey sfoster, do we know if this behaviour is expected to be behind a Proton pref, or is it able to ride out earlier?

I was assuming we wanted this behavior behind the proton pref and my WIP patch implements this, but in theory it could be separate.

Flags: needinfo?(sfoster)

Thanks, Sam. See bug 1690131 comment 3 - sounds like we'll want this behind the Proton pref as you're already doing it.

I'm not sure how we want to manage having the FxA toolbarbutton be visible/hidden, I'm watching to see how the patch on bug 1686523 plays out.
Maybe we do need a different placements list for proton. Anyhow I'll work up some tests in the meantime.

I suggest looking at how the Downloads button hides / shows itself. The patches in bug 1397447 might help give some ideas. But ultimately, I suspect that, per-window, this should probably be managed in browser-sync.js.

Right now, if you simply set the FxA button to be initially hidden, and toggle it when gSync.isSignedIn becomes true, nothing appears to happen as the FxA button is in the overflow.

So, I looked at the downloads button patches. They add a downloadsbuttonshown attribute to the #navbar when a download happens, and we have CSS to reserves some space for the button so stuff doesn't fall off into overflow when it appears. That is very similar to the behavior we want for the FxA button, but the downloads solution is very one-off/special-cased. I don't know how many toolbar buttons might want this behavior, but adding an attribute and CSS for each (and all their combinations) doesn't seem very scalable. However, if that looks like the right general approach I can try to extend or generalize it just enough to make it work here.

Flags: needinfo?(mconley)

Sorted this out with sfoster over Matrix. Basically, we're going to make the Firefox Accounts button non-overflowable by default when Proton is enabled, and set its visibility per window in browser-sync.js.

Flags: needinfo?(mconley)

(In reply to Mark Striemer [:mstriemer] from comment #0)

Once the user signs in, the button should be added to the toolbar automatically, but should still be customizable.

The FxA toolbar button is not currently customizable. Does you mean this patch should add it as a customizable widget a user could add permanently to their toolbar?

Flags: needinfo?(mstriemer)

(In reply to Sam Foster [:sfoster] (he/him) from comment #9)

(In reply to Mark Striemer [:mstriemer] from comment #0)

Once the user signs in, the button should be added to the toolbar automatically, but should still be customizable.

The FxA toolbar button is not currently customizable. Does you mean this patch should add it as a customizable widget a user could add permanently to their toolbar?

Scratch that. I had looked previously and didn't see the FxA button there in customize mode to add/remove to the toolbar. Not sure what was going on, but its there and I can remove it and add it again. Maybe the question is, should we do the same thing here as we do for downloads - have an option to auto-hide? Otherwise I'm not sure how a user would indicate they explicitly want the old behavior where the button is visible even if they aren't signed in.

Attachment #9204092 - Attachment description: Bug 1686521 (WIP) - Proton: Show FxA button based on signed-in state. r?Gijs → Bug 1686521 - Proton: Show FxA button based on signed-in state. r?markh
Flags: needinfo?(mstriemer)
Priority: -- → P1

As per guidance from Vicky, for tracking, we're marking all the bugs that people are working on as P1.

Blocks: 1694817
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/60c74da65c99
Proton: Show FxA button based on signed-in state. r=markh,mstriemer
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch
See Also: → 1695916

Verified that the FxA icon is moved in toolbar when in signed-in state. Tests were performed on Firefox 88.0b3 and Nightly 89.0a1 (2021-03-25) under Windows 10, macOS 10.15.7 and Ubuntu 20.04

Status: RESOLVED → VERIFIED
Regressions: 1701926
Regressions: 1703794
Regressions: 1706551
You need to log in before you can comment on or make changes to this bug.