1275089 - browser_permissions_urlFieldHidden.js is going to permafail when the Gecko version number is bumped to 50

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

I would love to hear some theories as to how this test is sensitive to the version number. Anyway, merge day is two weeks away :)

https://treeherder.mozilla.org/logviewer.html#?job_id=20972866&repo=try#L2191

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

I have a fix for this.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: MozReview Request: Bug 1275089 - Wrap the call to getHistogramById().add() in a try/catch since the probe expires in v50 and the histogram is no longer reported as a COUNT histogram since it is expired. Histograms that are not COUNT histograms must supply

Review commit: https://reviewboard.mozilla.org/r/55860/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55860/

Comment 3

[Matthew N. [:MattN]]

https://reviewboard.mozilla.org/r/55860/#review52634

::: browser/components/preferences/in-content/content.js:136
(Diff revision 1)
> +    try {
> -    Services.telemetry
> +      Services.telemetry
> -            .getHistogramById("WEB_NOTIFICATION_EXCEPTIONS_OPENED").add();
> +              .getHistogramById("WEB_NOTIFICATION_EXCEPTIONS_OPENED").add();
> +    } catch (e) {}

Did you consider postponing the expiry or removing the code instead?

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

I don't have the answer as to whether we should postpone or remove the code. Jeff or Edwin may want to jump in here.

As it stands, this is fine for 49 as it won't cause any harm and the code can be removed after the probe expires.

Comment 5

[:Gijs (he/him)]

Comment on attachment 8757423 [details]
MozReview Request: Bug 1275089 - Wrap the call to getHistogramById().add() in a try/catch since the probe expires in v50 and the histogram is no longer reported as a COUNT histogram since it is expired. Histograms that are not COUNT histograms must supply

https://reviewboard.mozilla.org/r/55860/#review52856

rs=me for the stopgap but please don't land until Friday. Would be nicer if we could just rm the code or extend the life of the histogram.

Comment 6

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Redirecting to Bryan

Comment 7

[Edwin Wong [:edwong]]

deferring to :clarkbw

Comment 8

[Bryan Clark (DevTools PM) [:clarkbw]]

I'm a little confused about the data this probe represents.

WEB_NOTIFICATION_EXCEPTIONS_OPENED: "Number of times the Notification Permissions dialog has been opened"
https://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/Histograms.json#10595

Aren't we getting the same information via this probe? (keyed to 'web-notifications' and value 0)

POPUP_NOTIFICATION_STATS
https://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/Histograms.json#5791

Comment 9

[Matthew N. [:MattN]]

This is about the subdialog in about:preferences#content to manage allowed/denied sites for notifications. That is different than popup notifications which are for the initial permission prompt.

Basically this is giving data on how often people manage their notification permissions and this pane is what we link to from the gear menu.

Comment 10

[Bryan Clark (DevTools PM) [:clarkbw]]

Thanks Matt!

In terms of just Push this could be useful to have.  But since its not opt-out already I would need it changed to opt-out and extended until 55 to give us time to incorporate it into our dashboards. 

I don't want to bounce this to another person, but I think Javaun is the one who might find value in that metric.  Unclear if it needs similar changes.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/346d46f452a5
Wrap the call to getHistogramById().add() in a try/catch since the probe expires in v50 and the histogram is no longer reported as a COUNT histogram since it is expired. Histograms that are not COUNT histograms must supply a value when calling .add(). r=Gijs

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/346d46f452a5