Closed Bug 1166462 Opened 5 years ago Closed 5 years ago

Add Telemetry probe for content of "Title bar" setting

Categories

(Firefox for Android :: Settings and Preferences, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 41
Tracking Status
firefox40 --- fixed
firefox41 + fixed
fennec 40+ ---

People

(Reporter: mcomella, Assigned: mcomella)

References

Details

Attachments

(2 files, 1 obsolete file)

Showing the page title for a page adds some complication in updating the toolbar state - we could simplify it (and the settings!) but not providing the option to show the page title and just showing the url, as we do by default.

We should get some numbers on how often this feature is used and thus if we can remove it. Currently, we can get the number of times this item was clicked, given the number of times Settings was accessed. Ideally, we find out how many users have "show title" enabled (see bug 1166461).
Anthony, would you be opposed to this?
Flags: needinfo?(alam)
Blocks: menu-reorg
Flags: needinfo?(alam)
^whoops

I'm open to it. But I'd like to think about it some more. I can see a lot of pros and cons right now and some telemetry might also help.

Keep in mind, depending on what the default we ship with is and what the user current has selected, just "how many users have "show title" enabled" wont really tell us much about the bigger picture.
Let's add some telemetry!
Assignee: nobody → michael.l.comella
Comment on attachment 8625796 [details] [diff] [review]
(WIP) Add probe

Margaret, this seems to fail with:

NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsITelemetry.getHistogramById]

Any ideas?
Attachment #8625796 - Flags: feedback?(margaret.leibovic)
(In reply to Michael Comella (:mcomella) from comment #5)
> Comment on attachment 8625796 [details] [diff] [review]
> (WIP) Add probe
> 
> Margaret, this seems to fail with:
> 
> NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057
> (NS_ERROR_ILLEGAL_VALUE) [nsITelemetry.getHistogramById]
> 
> Any ideas?

Did you build toolkit also?
(In reply to :Margaret Leibovic from comment #6)
> Did you build toolkit also?

I feel silly. :d
Bug 1166462 - Add probe for title in titlebar enabled setting. r=margaret
Attachment #8626388 - Flags: review?(margaret.leibovic)
Attachment #8625796 - Attachment is obsolete: true
Attachment #8625796 - Flags: feedback?(margaret.leibovic)
Comment on attachment 8626388 [details]
MozReview Request: Bug 1166462 - Add probe for title in titlebar enabled setting. r=margaret

https://reviewboard.mozilla.org/r/12087/#review10489

Ship It!
Attachment #8626388 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8626388 [details]
MozReview Request: Bug 1166462 - Add probe for title in titlebar enabled setting. r=margaret

https://reviewboard.mozilla.org/r/12087/#review10491

::: toolkit/components/telemetry/Histograms.json:8265
(Diff revision 1)
> +    "expires_in_version": "43",

If you land this today, you could actually make this expire in 42. We should also try uplifting this.
Attachment #8626388 - Flags: review+
(In reply to :Margaret Leibovic from comment #10)
> If you land this today, you could actually make this expire in 42. We should
> also try uplifting this.

Changed.
Comment on attachment 8626388 [details]
MozReview Request: Bug 1166462 - Add probe for title in titlebar enabled setting. r=margaret

Approval Request Comment
[Feature/regressing bug #]: bug 1166462.
[User impact if declined]:
  We'd have to wait longer to determine whether or not we can remove the title bar preference feature.

[Describe test coverage new/current, TreeHerder]:
  Tested locally w/ added print statements.

[Risks and why]: 
  Low - TRACKING_PROTECTION_ENABLED follows a similar paradigm so I just copied that.

[String/UUID change made/needed]: None
Attachment #8626388 - Flags: approval-mozilla-aurora?
(In reply to Michael Comella (:mcomella) from comment #12)
> (In reply to :Margaret Leibovic from comment #10)
> > If you land this today, you could actually make this expire in 42. We should
> > also try uplifting this.
> 
> Changed.

...but I forgot to land it. I'm just going to leave it 43.
Whiteboard: [leave-open]
Adding a tracking flag for FF41. Not sure why this is not tracking for Fennec either.
Comment on attachment 8626388 [details]
MozReview Request: Bug 1166462 - Add probe for title in titlebar enabled setting. r=margaret

Approving uplift to Aurora. This patch seems non-invasive and has been in m-c for a while now and we have not heard of any negative impact.
Attachment #8626388 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
tracking-fennec: --- → 41+
This landed on m-c prior to the uplift.
Comment on attachment 8626388 [details]
MozReview Request: Bug 1166462 - Add probe for title in titlebar enabled setting. r=margaret

Oh, actually, this was intended to be uplifted to 40.

Approval Request Comment 13
Attachment #8626388 - Flags: approval-mozilla-beta?
tracking-fennec: 41+ → 40+
Summary: Consider removing "Title bar" setting (i.e. show title or url) → Add Telemetry probe for content of "Title bar" setting
Comment on attachment 8626388 [details]
MozReview Request: Bug 1166462 - Add probe for title in titlebar enabled setting. r=margaret

Low risk and helps the team, taking it.
Attachment #8626388 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Needs rebasing for beta uplift.
Flags: needinfo?(michael.l.comella)
Here's a branch patch that I have yet to build - I'll test sometime between today and tomorrow and clear the NI.
Flags: needinfo?(michael.l.comella)
Branch patch seems to work:
  * does not crash on startup :)
  * can load pages (I believe JS threw an error when I didn't add the histogram when initially developing this and prevented this)
  * I locally added a print statement after the Telemetry.addData method and it prints out, meaning the method call is a success

NI Ryan to land (because I don't know if this would be seen otherwise).
Flags: needinfo?(ryanvm)
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [leave-open]
Target Milestone: --- → Firefox 41
Flags: needinfo?(ryanvm)
You need to log in before you can comment on or make changes to this bug.