Bug 1901797 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.

How do we make it so that, if you forget to specify `!activeNotifications` in your message, and there is an active notification, the message will not show? If my targeting is just `source == 'newtab'`, and FxMS treats that expression _as if_ it includes `&& !activeNotifications`, then how do I make a message do the opposite, i.e. make it allow collisions? We could make it so FxMS leaves the expression alone if it already contains the string `activeNotifications`. But I can't add the opposite (`activeNotifications`) to my targeting expression, since that doesn't mean "ignore active notifications", it means "only show if there _are_ active notifications."  I could put `activeNotifications || !activeNotifications` in the targeting to deal with that issue, but that's really awkward and a likely footgun.

This is fundamentally a difficult thing to handle in targeting, because a targeting expression is a series of conditions for when _not_ to show the message, not for when _to_ show the message.

If instead of targeting this was just a new property like this:

```
allow_collisions?: boolean
```

Then you'd be able to specify true to override the default behavior, or just leave it undefined to disallow collisions. This is no harder than implementing this in targeting, no more lines of code at least and cuts past all this targeting confusion.

The main problem I can see is that we'd effectively be changing EVERY message that already exists. The only way to know this won't break something is to consider everything. The work here isn't so much writing code, but checking at least every type of message to see if it's supposed to be able to show while the urlbar or an infobar is showing. For the bigger ones, it's obvious. But what about ToastNotification? Moments Pages? Private Browsing Newtab? I think this could also be problematic for non-doorhanger CFRs. Probably others I'm not thinking of. Thankfully it won't create future work for us, since we'll see during development that a message isn't showing when we think it's supposed to, and we'll catch that we need to add `allow_collisions` to that message (or specify a rule for specific templates).
How do we make it so that, if you forget to specify `!activeNotifications` in your message, and there is an active notification, the message will not show? If my targeting is just `source == 'newtab'`, and FxMS treats that expression _as if_ it includes `&& !activeNotifications`, then how do I make a message do the opposite, i.e. make it allow collisions? We could make it so FxMS leaves the expression alone if it already contains the string `activeNotifications`. But I can't add the opposite (`activeNotifications`) to my targeting expression, since that doesn't mean "ignore active notifications", it means "only show if there _are_ active notifications."  I could put `activeNotifications || !activeNotifications` in the targeting to deal with that issue, but that's really awkward and a likely footgun.

This is fundamentally a difficult thing to handle in targeting, because a targeting expression is a series of conditions for when _not_ to show the message, not for when _to_ show the message.

If instead of targeting this was just a new property like this:

```
allow_collisions?: boolean
```

Then you'd be able to specify true to override the default behavior, or just leave it undefined to disallow collisions. And of course you'd need to add some code in ASRouterTargeting to check the property and the current value for `activeNotifications`. This is no harder than implementing this in targeting, no more lines of code at least and cuts past all this targeting confusion.

The main problem I can see is that we'd effectively be changing EVERY message that already exists. The only way to know this won't break something is to consider everything. The work here isn't so much writing code, but checking at least every type of message to see if it's supposed to be able to show while the urlbar or an infobar is showing. For the bigger ones, it's obvious. But what about ToastNotification? Moments Pages? Private Browsing Newtab? I think this could also be problematic for non-doorhanger CFRs. Probably others I'm not thinking of. Thankfully it won't create future work for us, since we'll see during development that a message isn't showing when we think it's supposed to, and we'll catch that we need to add `allow_collisions` to that message (or specify a rule for specific templates).
How do we make it so that, if you forget to specify `!activeNotifications` in your message, and there is an active notification, the message will not show? If my targeting is just `source == 'newtab'`, and FxMS treats that expression _as if_ it includes `&& !activeNotifications`, then how do I make a message do the opposite, i.e. make it allow collisions? We could make it so FxMS leaves the expression alone if it already contains the string `activeNotifications`. But I can't add the opposite (`activeNotifications`) to my targeting expression, since that doesn't mean "ignore active notifications", it means "only show if there _are_ active notifications."  I could put `activeNotifications || !activeNotifications` in the targeting to deal with that issue, but that's really awkward and a likely footgun.

This is fundamentally a difficult thing to handle in targeting, because a targeting expression is a series of conditions for when _not_ to show the message, not for when _to_ show the message.

If instead of targeting this was just a new property like this:

```
allow_collisions?: boolean
```

Then you'd be able to specify true to override the default behavior, or just leave it undefined to disallow collisions. And of course you'd need to add some code in ASRouterTargeting to check `message.allow_collisions || !this.activeNotifications`. This is no harder than implementing this in targeting, no more lines of code at least and cuts past all this targeting confusion.

The main problem I can see is that we'd effectively be changing EVERY message that already exists. The only way to know this won't break something is to consider everything. The work here isn't so much writing code, but checking at least every type of message to see if it's supposed to be able to show while the urlbar or an infobar is showing. For the bigger ones, it's obvious. But what about ToastNotification? Moments Pages? Private Browsing Newtab? I think this could also be problematic for non-doorhanger CFRs. Probably others I'm not thinking of. Thankfully it won't create future work for us, since we'll see during development that a message isn't showing when we think it's supposed to, and we'll catch that we need to add `allow_collisions` to that message (or specify a rule for specific templates).
How do we make it so that, if you forget to specify `!activeNotifications` in your message, and there is an active notification, the message will not show? If my targeting is just `source == 'newtab'`, and FxMS treats that expression _as if_ it includes `&& !activeNotifications`, then how do I make a message do the opposite, i.e. make it allow collisions? We could make it so FxMS leaves the expression alone if it already contains the string `activeNotifications`. But I can't add the opposite (`activeNotifications`) to my targeting expression, since that doesn't mean "ignore active notifications", it means "only show if there _are_ active notifications."  I could put `activeNotifications || !activeNotifications` in the targeting to deal with that issue, but that's really awkward and a likely footgun.

This is fundamentally a difficult thing to handle in targeting, because a targeting expression is a series of conditions for when _not_ to show the message, not for when _to_ show the message.

If instead of targeting this was just a new property like this:

```
allow_collisions?: boolean
```

Then you'd be able to specify true to override the default behavior, or just leave it undefined to disallow collisions. And of course you'd need to add some code in ASRouterTargeting to check `message.allow_collisions || !this.activeNotifications`. This is no harder than any other approach to implementation we could take, no more lines of code at least and cuts past all the confusion from the first paragraph.

The main problem I can see is that we'd effectively be changing EVERY message that already exists. The only way to know this won't break something is to consider everything. The work here isn't so much writing code, but checking at least every type of message to see if it's supposed to be able to show while the urlbar or an infobar is showing. For the bigger ones, it's obvious. But what about ToastNotification? Moments Pages? Private Browsing Newtab? I think this could also be problematic for non-doorhanger CFRs. Probably others I'm not thinking of. Thankfully it won't create future work for us, since we'll see during development that a message isn't showing when we think it's supposed to, and we'll catch that we need to add `allow_collisions` to that message (or specify a rule for specific templates).

Back to Bug 1901797 Comment 5