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).
Bug 1825381 Comment 5 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
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).
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
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?
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. 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 think Spotlight is probably the closest fit, 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.
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, 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.
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.