Bugzilla

Filed in Messaging for tracking purposes; will move to Networking later.

In bug 1722834, telemetry was added for a few things, including when the captive-portal-login-success observer notification was fired, and when the captive portal login infobar was shown.  

Unfortunately, after doing some redash analysis on our recent Nightly A:A experiment, it became clear that the login-success notifications weren't always being sent.  

After some code inspection, it became clear that the telemetry was being sent in the only when the captive portal infobar was made to disappear, and never if the captive portal infobar hadn't been shown.  I've got a patch that moves it from an observer to the code in CaptiveDetect.jsm where the notification is being fired, which should be more correct, and should less fragile, since it doesn't depend on when (for example) the observer gets registered.

I also took a look at the other telemetry, and the captive portal login infobar telemetry could be sent whenever the _showNotification method (which sometimes returns early) was called, meaning potentially too many times.  I pushed this notification down into the _showNotification method to the spot after the notification is actually shown.

I'd like to get this in on Monday if at all possible, as I'm hoping to get this uplifted to 92 so that we can run a 92 release A:A experiment to validate our telemetry if necessary.





these two are fired in the wrong places.  captive-portal-login-succes

Filed in Messaging for tracking purposes; will move to Networking later.

In bug 1722834, telemetry was added for a few things, including when the captive-portal-login-success observer notification was fired, and when the captive portal login infobar was shown.  

Unfortunately, after doing some redash analysis on our recent Nightly A:A experiment, we saw that the login-success notifications weren't always being sent.  

After some code inspection, it became clear that the telemetry was being sent only when the captive portal infobar was made to disappear, and never if the captive portal infobar hadn't been shown.  I've got a patch that moves it from an observer to the code into CaptiveDetect.jsm where the notification is being fired, which should be more correct and also less fragile, since it doesn't depend on when (for example) the observer gets registered.

I also took a look at the other telemetry, and the captive portal login infobar telemetry could be sent whenever the _showNotification method (which sometimes returns early) was called, meaning potentially too many times.  I pushed this notification down into the _showNotification method to the spot after the notification is actually shown.

I'd like to get this in on Monday if at all possible, as ideally, I'd like to get this uplifted to 92 so that we can run a 92 release A:A experiment to validate our telemetry if necessary.





these two are fired in the wrong places.  captive-portal-login-succes

Filed in Messaging for tracking purposes; will move to Networking later.

In bug 1722834, telemetry was added for a few things, including when the captive-portal-login-success observer notification was fired, and when the captive portal login infobar was shown.  

Unfortunately, after doing some redash analysis on our recent Nightly A:A experiment, we saw that the login-success notifications weren't always being sent.  

After some code inspection, it became clear that the telemetry was being sent only when the captive portal infobar was made to disappear, and never if the captive portal infobar hadn't been shown.  I've got a patch that moves it from an observer to the code into CaptiveDetect.jsm where the notification is being fired, which should be more correct and also less fragile, since it doesn't depend on when (for example) the observer gets registered.

I also took a look at the other telemetry, and the captive portal login infobar telemetry could be sent whenever the _showNotification method (which sometimes returns early) was called, meaning potentially too many times.  I pushed this notification down into the _showNotification method to the spot after the notification is actually shown.

I'd like to get this in on Monday if at all possible, as ideally, I'd like to get this uplifted to 92 so that we can run a 92 release A:A experiment to validate our telemetry if necessary.