Bug 1561540 - CFR messages should have a priority field that allows sorting #5156
Conversation
piatra
force-pushed
the
bug1561349
branch
3 times, most recently
from
95079fd
to
bcaae99
Compare
|
this will need to be rebased again and have the formatting fixed with prettier |
| @@ -771,6 +780,10 @@ class _ASRouter { | |||
|
|
|||
| uninit() { | |||
| this._storage.set("previousSessionEnd", Date.now()); | |||
| this._storage.set( | |||
| "previousSessionFirefoxVersion", | |||
| ASRouterTargeting.Environment.firefoxVersion | |||
There was a problem hiding this comment.
I think we said we might want to uplift this to 69 right? Or will we handle the special case of undefined and assume that means an upgrade happened?
There was a problem hiding this comment.
I'm not sure anymore this is the best way to do it. For example it will not cover users that upgrade from 68 to 70.
I think a better targeting expression would be
{
"id": "WHATS_NEW_BADGE_${FIREFOX_VERSION}",
"template": "toolbar_badge",
...
"targeting": "firefoxVersion >= 70 && ..."I think this will work:
- We know the feature will be in 70 and we want to show it for everyone starting with 70
- It will work for 70+
- Because of the priority field we know it will be picked before other badges
WHATS_NEW_BADGE_70won't be shown after 4 days because of targeting + impressionsWHATS_NEW_BADGE_71will match again because of the different impression key- And we don't need to update the targeting for this or publish a new message
- It's independent of the what's new messages (they can have different targeting expressions that decide what Fx 70 users see vs Fx 71)
Let me know what you think, am I missing any case here?
There was a problem hiding this comment.
Ah, but we want to show whats's new to users that upgrade to 70 not users that first run 70.
There was a problem hiding this comment.
We can use TelemetryReportingPolicy.isFirstRun() to detect first runs in 70 (new users). We can skip what's new notifications for a whole release for these users by setting some pref/indexedDB value. This should work around the need to land before 70 constraint and work for users that upgrade from an older version.
|
oh, looks like an aboutwelcome mochitest failed |
…orting (mozilla#5156) Resolve conflicts from not uplifting ToolbarPanelHub 025e6d2
…orting (mozilla#5156) Resolve conflicts from not uplifting ToolbarPanelHub 025e6d2
No description provided.