Move "Set to default" notification to Messaging System
Categories
(Firefox :: Messaging System, task, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox134 | --- | fixed |
People
(Reporter: vtay, Assigned: hanna_a)
References
(Depends on 2 open bugs, Blocks 3 open bugs)
Details
Attachments
(6 files)
Move (or remove) "Set to default" notification messages to AS Router (there appears to be 2 - one is "primary" browser" and another is "set to default").
If/When we move them into AS Router, they should be part of a "set to default" group so that we can control collisions.
A couple of questions that will help us determine next steps:
- Might we have any data on the engagement with these notification messages?
- I believe this messaging component does not exist in AS Router - do we need it? When might we use it?
Comment on attachment 9325800 [details]
Screen Shot 2023-03-29 at 10.32.38 AM.png
"Make Firefox your primary browser"
Comment on attachment 9325801 [details]
image (43).png
"Make Firefox your default browser"
Comment 4•3 years ago
|
||
Notification shown with content switched based off browser needPin status :
https://searchfox.org/mozilla-central/source/browser/locales/en-US/browser/defaultBrowserNotification.ftl#19
https://searchfox.org/mozilla-central/rev/0907529ff72c456ddb47839f5f7ba16291f28dce/browser/components/BrowserGlue.sys.mjs#5416
Comment 5•3 years ago
•
|
||
Just by virtue of converting these to ASRouter messages and making them trigger on defaultBrowserCheck will already prevent collisions, since there's only one trigger per startup, and we only route one message per trigger (the first matched by priority/weighting/insertion order). So the group may be unnecessary, though it still might be worth doing if we want to also prevent users from just seeing too many "set to default" affordances in general. That seems like maybe something to consider in its own bug though, as it affects many other messages, and poses the question of what to do with multi-screen messages that contain a "set to default" CTA (like about:welcome and the infobar in bug 1825377).
Also if we want to move these into ASRouter instead of removing them, we will need to do something about the strings too. Seems a bit overkill to insert them into spotlights (conditionally or not). I think we can use a Fluent migration to simply transfer them to onboarding.ftl
Edit: just realized we've been sending the wrong telemetry for this startup check since 2019:
isDefault = shellService.isDefaultBrowser(isStartupCheck, false);
Services.telemetry
.getHistogramById("BROWSER_IS_USER_DEFAULT")
.add(isDefault);
The first param isStartupCheck was removed in bug 1521902. So we're actually passing true for the first param, which is now aForAllTypes (what the 2nd param used to be, and which we meant to passing false for). Which is potentially skewing the data?
One other thing - this message uses the prompt service and we don't have a messaging surface for that. (It wouldn't be worth making one since we can do much more with extending Spotlight than investing in the old prompt service.) So there isn't a simple way to move this into our system with its visual appearance fully intact.
Spotlights are similar but the content layout is a little different - spotlight's images are more illustrations than little logo icons, and the content is centered instead of left aligned. CFRs have similar content layout, but they're more popups than dialogs, i.e. they don't block interaction outside of the popup, they don't draw a backdrop overlay on the window, and they're anchored to other elements.
I guess Spotlight is probably the closest fit since there's no obvious element to anchor it to (maybe the app menu hamburger button, but that places it way off to the right), but either way the end result is so different from the message as it currently exists that I'm not sure it's worth trying to replicate it, and maybe it's better to just make a new message if/when we want one.
Comment 6•3 years ago
|
||
Decision based on @vtay conversation this afternoon
Updated•2 years ago
|
Comment 7•2 years ago
|
||
I found another message we might want to move into the messaging system. See code here.
@shane That looks like an easy migration to info bar?
I realised I dropped the call on the moving set-to-default messages over.
What is the next step? Use spotlights to mimic the current panel?
Comment 9•2 years ago
|
||
(In reply to Vtay from comment #8)
I realised I dropped the call on the moving set-to-default messages over.
What is the next step? Use spotlights to mimic the current panel?
Not at all - we had a meeting about it and agreed that, for now, it isn't a critical issue because there is already a way to make sure our defaultBrowserCheck (startup) messages don't conflict with the hard-coded set-to-default message. We just have to add an extra attribute to the message targeting when we want to do a startup message.
As for next steps, I think this bug can be tackled by trying to reconstruct the message in Spotlight. There are other possibilities but I think we'll get the most long-term bang for our buck by adding whatever style and targeting features are needed to recreate the message (at maybe 90% accuracy) in Spotlight. It may be hard to get it to look exactly identical, but I figure you'd agree there's some wiggle room with respect to the visual style and some of the targeting nuances.
Right now, the most difficult part is probably that the set-to-default message uses pretty elaborate "targeting" rules. It doesn't use the messaging system for targeting, it just has some code that decides whether this message will show or not. Some of the parameters have existing targeting attributes, but at a glance it looks like some new targeting capabilities will need to be added (on train). But the engineer assigned to the bug can figure that out.
@shane That looks like an easy migration to info bar?
We can definitely make a visually identical message quite easily with InfoBar. But, as with the set-to-default message, the difficulty here is in recreating the complicated targeting in JEXL (the targeting format used by the messaging system). So it would be another thing an engineer will have to look at and most likely land some code changes.
Some notes for engineers...
These messages' maybeShow methods both read and set prefs, but it looks like those prefs are basically used to create one-off frequency caps. We should be able to remove those prefs if we can use frequency caps instead. Though we might want to do some kind of migration from pref values to ASRouter storage, so that users who already have seen the old hard-coded messages do not start seeing the new FxMS versions.
Other parameters look like they require new targeting attributes. See e.g. SessionStore.canRestoreLastSession and SessionStartup.sessionType == SessionStartup.RECOVER_SESSION
| Assignee | ||
Updated•1 year ago
|
| Reporter | ||
Comment 11•1 year ago
|
||
In the event that:
- User has Firefox set to default
- Users unsets Firefox to default (by setting up another browser to default)
- User starts-up Firefox
Currently - user will not see the message.
We should change this and have the user sees the message again.
| Reporter | ||
Comment 12•1 year ago
|
||
Note that checkbox corresponds to an about:setting preference "Always check if Firefox is your default browser"
Updated•1 year ago
|
| Assignee | ||
Comment 13•1 year ago
|
||
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
| Assignee | ||
Comment 14•1 year ago
|
||
Initial experiment brief: https://docs.google.com/document/d/1u4msyp6z7DDYnLMK3Pk3rHENTr429RzOYF1HU-BCueg/edit?tab=t.0
| Assignee | ||
Updated•1 year ago
|
Updated•1 year ago
|
Comment 15•1 year ago
|
||
Comment 16•1 year ago
|
||
| bugherder | ||
Comment 17•1 year ago
|
||
Comment 18•1 year ago
•
|
||
Attaching a screenshot of the default prompt on Mac to highlight the OS-level difference in the position of the primary button (displayed to the right of the secondary action).
Updated•1 year ago
|
Description
•