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

RESOLVED FIXED in Firefox 50

Status

()

--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: RyanVM, Assigned: jaws)

Tracking

Trunk
Firefox 50
Points:
---

Firefox Tracking Flags

(firefox49 affected, firefox50 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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
Flags: needinfo?(jaws)
I have a fix for this.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Flags: needinfo?(jaws)
Created 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

Review commit: https://reviewboard.mozilla.org/r/55860/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55860/
Attachment #8757423 - Flags: review?(gijskruitbosch+bugs)
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?
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.
Flags: needinfo?(jgriffiths)
Flags: needinfo?(edwong)

Comment 5

2 years ago
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.
Attachment #8757423 - Flags: review?(gijskruitbosch+bugs) → review+
Redirecting to Bryan
Flags: needinfo?(jgriffiths) → needinfo?(clarkbw)

Comment 7

2 years ago
deferring to :clarkbw
Flags: needinfo?(edwong)
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
Flags: needinfo?(clarkbw)
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.
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.
Flags: needinfo?(jmoradi)

Comment 11

2 years ago
Pushed by jwein@mozilla.com:
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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/346d46f452a5
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Flags: needinfo?(jmoradi)
You need to log in before you can comment on or make changes to this bug.