Record badge shown and doorhanger shown in separate telemetry histograms

RESOLVED FIXED in Firefox 55

Status

()

P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: rstrong, Assigned: dthayer)

Tracking

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

Both of these are currently recorded in update_notification_shown. I suggest that update_notification_shown is used for doorhangers since only doorhangers can be dismissed which is already recorded in the update_notification_dismissed histogram. The new histogram for badges being shown could be named update_notification_badge_shown.
(Assignee)

Comment 1

2 years ago
To clarify, update_notification_badge_shown would only be recorded if the badge came up by itself, correct? (i.e., not by dismissing a doorhanger.)
As long as all cases where the badge is shown before the doorhanger will be recorded as update_notification_badge_shown this should be fine. I don't *think* we want to record update_notification_badge_shown for the case when it is shown due to the the doorhanger being dismissed (e.g. update_notification_dismissed), etc. Does that make sense to you?
Flags: needinfo?(dothayer)
(Assignee)

Comment 3

2 years ago
Yup! Sounds good to me.
Flags: needinfo?(dothayer)
Doug, could you take this bug?
Flags: needinfo?(dothayer)
(Assignee)

Comment 5

2 years ago
Yup! I was just waiting for 1359733 to land.
Assignee: nobody → dothayer
Flags: needinfo?(dothayer)
Comment hidden (mozreview-request)
Comment on attachment 8870895 [details]
Bug 1365204 - Add update badge histogram

https://reviewboard.mozilla.org/r/142468/#review146230

::: toolkit/components/telemetry/Histograms.json:5832
(Diff revision 1)
>      "bug_numbers": [893505],
>      "releaseChannelCollection": "opt-out",
>      "description": "Update: the application update doorhanger type that was displayed.",
>      "labels": ["restart", "available", "manual"]
>    },
> +  "UPDATE_NOTIFICATION_BADGE_SHOWN": {

You will need to get a data steward (e.g. bsmedberg, etc.) to approve the new telemetry.

::: toolkit/mozapps/update/UpdateListener.jsm:104
(Diff revision 1)
>  
>      AppMenuNotifications.showNotification("update-" + type,
>                                            action,
>                                            secondaryAction,
>                                            { dismissed, beforeShowDoorhanger });
> +    if (dismissed) {

Took me a minute to realize that dismissed means show the badge and not the doorhanger vs. the doorhanger being dismissed. If I have this wrong please let me know.

Please file a bug for adding java comments for the functions in this file.
Attachment #8870895 - Flags: review?(robert.strong.bugs) → review+
(Assignee)

Comment 8

2 years ago
mozreview-review-reply
Comment on attachment 8870895 [details]
Bug 1365204 - Add update badge histogram

https://reviewboard.mozilla.org/r/142468/#review146230

> Took me a minute to realize that dismissed means show the badge and not the doorhanger vs. the doorhanger being dismissed. If I have this wrong please let me know.
> 
> Please file a bug for adding java comments for the functions in this file.

Added https://bugzilla.mozilla.org/show_bug.cgi?id=1367778
(Assignee)

Updated

2 years ago
Attachment #8870895 - Flags: review?(benjamin)

Comment 9

2 years ago
mozreview-review
Comment on attachment 8870895 [details]
Bug 1365204 - Add update badge histogram

https://reviewboard.mozilla.org/r/142468/#review147924

data-r=me (this is a followup to the data from bug 893505, and we're using the data analysis plan described there)
Attachment #8870895 - Flags: review?(benjamin) → review+

Comment 10

2 years ago
Pushed by dothayer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/860549ef693b
Add update badge histogram r=bsmedberg,rstrong

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/860549ef693b
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.