Closed
Bug 1329156
Opened 7 years ago
Closed 7 years ago
Custom tabs: Collect information on custom tab customization features used by 3rd party apps
Categories
(Firefox for Android Graveyard :: General, defect)
Firefox for Android Graveyard
General
Tracking
(firefox55 fixed)
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: jcheng, Assigned: cnevinchen)
References
Details
Attachments
(1 file, 1 obsolete file)
As Mozilla, we want to understand which custom tab customization features are being used by 3rd party apps
Reporter | ||
Updated•7 years ago
|
Blocks: customtabs
Comment 1•7 years ago
|
||
After main feature of custom tab is landed and completed, this will be implemented to collect telemetry information.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8857306 [details] Bug 1329156 - Refactor: Utility class should be final. https://reviewboard.mozilla.org/r/129266/#review131976 Not sure of the win here without a private constructor though?
Attachment #8857306 -
Flags: review?(s.kaspari) → review+
Updated•7 years ago
|
Attachment #8857307 -
Flags: review?(s.kaspari) → review?(walkingice0204)
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8857307 [details] Bug 1329156 - Add telemetry for customtab customization usage. https://reviewboard.mozilla.org/r/129268/#review131978 ::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java:254 (Diff revision 1) > + try { > + String extras = getResources().getResourceEntryName(item.getItemId()); > + Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.MENU, "customtab-" + extras); > + } catch (Resources.NotFoundException e) { > + Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.MENU, "customtab-custom-menu"); > + } I'd prefer us to be more explicit and add telemetry requests to the specific cases. Otherwise we might end up sending telemetry for things we didn't want to - especially in future iterations.
Comment 6•7 years ago
|
||
I've forwarded the review request to :walkingice. I consider him the owner of the custom tabs code and he will know best how/where to integrate the probes. :)
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8857307 [details] Bug 1329156 - Add telemetry for customtab customization usage. https://reviewboard.mozilla.org/r/129268/#review132840 ::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java:98 (Diff revision 1) > > actionBarPresenter = new ActionBarPresenter(actionBar); > actionBarPresenter.displayUrlOnly(startIntent.getDataString()); > + if (IntentUtil.hasToolbarColor(startIntent)) { > + Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.INTENT, "customtab-hasToolbarColor"); > + } Maybe we can create a method which contains Telemetry initialization checking. The method can be placed in activity first run[1]. Then "customtab-hasToolbarColor", "customtab-hasActionButton", "isActionButtonTinted"...etc can be aggregated in one place. [1] https://dxr.mozilla.org/mozilla-central/rev/f40e24f40b4c4556944c762d4764eace261297f5/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java#73 ::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java:254 (Diff revision 1) > + try { > + String extras = getResources().getResourceEntryName(item.getItemId()); > + Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.MENU, "customtab-" + extras); > + } catch (Resources.NotFoundException e) { > + Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.MENU, "customtab-custom-menu"); > + } I agree with Sebastian. The menu item id is generated by code so it is supposed to be meaningless for us. If we care about whether user click to trigger pending-intent, we can do something like ``` final PendingIntent intent = menuItemsIntent.get(item.getItemId()); if (intent != null) { onCustomMenuItemClicked(intent); return true; } ..... private void onCustomMenuItemClicked(PendingIntent intent) { // send Telemetry here for custom-menu-item performPendingIntent(intent); } ``` ::: mobile/android/base/java/org/mozilla/gecko/customtabs/IntentUtil.java:102 (Diff revision 1) > + * @param intent which to launch a Custom-Tabs-Activity > + * @return true, if the caller customized the color. > + */ > + static boolean hasToolbarColor(@NonNull Intent intent) { > + @ColorInt int toolbarColor = intent.getIntExtra(CustomTabsIntent.EXTRA_TOOLBAR_COLOR, > + Integer.MAX_VALUE); what if we use intent.hasExtra()?
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → cnevinchen
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8857306 -
Attachment is obsolete: true
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8857307 [details] Bug 1329156 - Add telemetry for customtab customization usage. https://reviewboard.mozilla.org/r/129268/#review136788
Attachment #8857307 -
Flags: review?(walkingice0204) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 11•7 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4847c0e2a3e1 Add telemetry for customtab customization usage. r=walkingice
Keywords: checkin-needed
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4847c0e2a3e1
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•