Closed Bug 1449212 Opened 7 years ago Closed 7 years ago

Add enabled accessibility services to core ping.

Categories

(Firefox for Android Graveyard :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox60 fixed, firefox61 fixed)

RESOLVED FIXED
Firefox 61
Tracking Status
firefox60 --- fixed
firefox61 --- fixed

People

(Reporter: yzen, Assigned: yzen)

References

Details

Attachments

(3 files, 1 obsolete file)

In order prioritize android accessibility work, specifically work on GeckoView accessibility, but also Fennec, we would like to learn more about what accessibility services our users use in order to tackle their support effectively.
Attached patch 1449212 patch (obsolete) — Splinter Review
The main idea is to gather information about all possible accessibility services that can interact with our android products (fennec, klar, etc). Such services include TalkBack, BrailleBack, SwitchAccess, etc. Information collected seems to fall into Category 1 “Technical data” since it only includes information about 3rd party software that interacts with our android products during usage. Accessibility service ids will help us determine how to prioritize accessibility support in GeckoView, primarily.
Attachment #8962855 - Flags: review?(sdaswani)
[triage] Susheel, please assign a priority to this bug.
Flags: needinfo?(sdaswani)
Yura, have you tested if your patch works?
Flags: needinfo?(sdaswani) → needinfo?(yzenevich)
(In reply to :sdaswani from comment #3) > Yura, have you tested if your patch works? I have tested the id collection part on the Moto G5 and it works well. I was wondering if there's a way to trigger the core ping builder locally, would you know, Susheel?
Flags: needinfo?(yzenevich) → needinfo?(sdaswani)
Comment on attachment 8962855 [details] [diff] [review] 1449212 patch The patch looks straightforward but I wonder if you should check if accessibilityManager is null? I assume enabledServices could never be null if accessibilityManager is non-null but a check may be worthwhile there? Otherwise it’s a r+.
Flags: needinfo?(sdaswani) → needinfo?(yzenevich)
Attached patch 1449212 patch v2Splinter Review
With the null check.
Attachment #8962855 - Attachment is obsolete: true
Attachment #8962855 - Flags: review?(sdaswani)
Flags: needinfo?(yzenevich)
Attachment #8963241 - Flags: review?(sdaswani)
Attachment #8963241 - Flags: review?(sdaswani) → review+
Attachment #8963276 - Flags: review?(sdaswani)
Attachment #8963276 - Flags: review?(sdaswani) → review+
For a review request: Data Collected This information will be added to the core ping, only if there are any accessibility services enabled at the time of the ping being built. Generally accessibility users use these services persistently (screen reader, magnifier etc). We collect ids of the accessibility services only. Just having the id's gives us enough information since, for each service, we know its features and capabilities. Purpose of Collection We would like to know which accessibility services interact with our android products. This would be very helpful to prioritize accessibility support work, primarily in GeckoView. Knowing which accessibility services interact with our android products will help us figure out how many users (in aggregate) use speech, haptic, braille feedback, explore by touch, etc. Data review request: https://gist.github.com/yzen/61f37e635b656a919eceea51311e8df7
Flags: needinfo?(liuche)
Examples of service IDs are as follows: For TalkBack: com.google.android.marvin.talkback/.TalkBackService For Switch Access: com.google.android.marvin.talkback/com.android.switchaccess.SwitchAccessService
> I have tested the id collection part on the Moto G5 and it works well. I was > wondering if there's a way to trigger the core ping builder locally TelemetryCorePingDelegate schedules an upload in onStart. `execute` triggers a ping build: https://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryCorePingDelegate.java#122
Comment on attachment 8963241 [details] [diff] [review] 1449212 patch v2 Review of attachment 8963241 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryCorePingDelegate.java @@ +186,5 @@ > if (campaignId != null) { > pingBuilder.setOptCampaignId(campaignId); > } > + > + pingBuilder.setOptAccessibility(context); Nit: I'd follow the pattern above of being explicit when the optional categories are added to the ping, rather than hiding that statement away in the function. That way the function behavior is minimized - passing in the whole Context opens this up to bad coding practices in the future.
Data-review only > Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate? Yes, intent to document in https://github.com/mozilla-services/mozilla-pipeline-schemas/blob/dev/schemas/telemetry/core/core.9.parquetmr.txt and present in coreping.rst > Is there a control mechanism that allows the user to turn the data collection on and off? (Note, for data collection not needed for security purposes, Mozilla provides such a control mechanism) Provide details as to the control mechanism available. Yes, Focus/Klar data controls > If the request is for permanent data collection, is there someone who will monitor the data over time? permanent, Yura Zenevich [:yzen] > Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under? Type 2 > Is the data collection request for default-on or default-off? default on > Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)? accessibility package names > Is the data collection covered by the existing Firefox privacy notice? Yes > Does there need to be a check-in in the future to determine whether to renew the data? (Yes/No) (If yes, set a todo reminder or file a bug if appropriate) No
Flags: needinfo?(liuche)
Pushed by yura.zenevich@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8ab320324b9f adding enabled accessibility services information to core ping. r=sdaswani https://hg.mozilla.org/integration/mozilla-inbound/rev/860ec10b301b update core ping docs describing the new accessibilityServices value. r=sdaswani
Comment on attachment 8963241 [details] [diff] [review] 1449212 patch v2 This patch adds new optional telemetry data to the core ping for all android products. We would like to know which accessibility services interact with our android products. This would be very helpful to prioritize accessibility support work (one of the main blockers) for GeckoView. [Feature/Bug causing the regression]: Not a regression [User impact if declined]: Accessibility and GeckoView teams will have not data about what accessibility services our users with disabilities use for another release cycle. [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: On inbound ATM [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: Only adds optional telemetry to the core ping, nothing user facing. [String changes made/needed]: None
Attachment #8963241 - Flags: approval-mozilla-beta?
Comment on attachment 8963276 [details] [diff] [review] 1449212 docs patch This is just a document specific patch for the code changes in the other one. [Feature/Bug causing the regression]: Not a regression, update to docs only. [User impact if declined]: This is just a documentation update with changes in the other patch. [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: On inbound ATM [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: Documentation change only. [String changes made/needed]: None
Attachment #8963276 - Flags: approval-mozilla-beta?
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Comment on attachment 8963241 [details] [diff] [review] 1449212 patch v2 Improve Fennec a11y telemetry, approved for 60.0b11.
Attachment #8963241 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8963276 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
nit in docs patch: s/out products/our products/
Flags: needinfo?(yzenevich)
Status: RESOLVED → REOPENED
Flags: needinfo?(yzenevich)
Resolution: FIXED → ---
Attachment #8967484 - Flags: review?(sdaswani) → review+
Pushed by yura.zenevich@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/44ef3e7ea4d8 fixing the typo in core-ping.rst doc. r=sdaswani
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: