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)
Firefox for Android Graveyard
General
Tracking
(firefox60 fixed, firefox61 fixed)
RESOLVED
FIXED
Firefox 61
People
(Reporter: yzen, Assigned: yzen)
References
Details
Attachments
(3 files, 1 obsolete file)
5.94 KB,
patch
|
sdaswani
:
review+
RyanVM
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
3.33 KB,
patch
|
sdaswani
:
review+
RyanVM
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
1.29 KB,
patch
|
sdaswani
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•7 years ago
|
||
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)
Assignee | ||
Comment 4•7 years ago
|
||
(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)
Assignee | ||
Comment 6•7 years ago
|
||
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+
Assignee | ||
Comment 7•7 years ago
|
||
Also updated the docs, and will update https://github.com/mozilla-services/mozilla-pipeline-schemas
Attachment #8963276 -
Flags: review?(sdaswani)
Attachment #8963276 -
Flags: review?(sdaswani) → review+
Assignee | ||
Comment 8•7 years ago
|
||
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)
Assignee | ||
Comment 10•7 years ago
|
||
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
Comment 11•7 years ago
|
||
> 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 12•7 years ago
|
||
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.
Comment 13•7 years ago
|
||
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)
Comment 14•7 years ago
|
||
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
Assignee | ||
Comment 15•7 years ago
|
||
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?
Assignee | ||
Comment 16•7 years ago
|
||
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?
Comment 17•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8ab320324b9f
https://hg.mozilla.org/mozilla-central/rev/860ec10b301b
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Comment 18•7 years ago
|
||
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+
Updated•7 years ago
|
Attachment #8963276 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 19•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/8f15181dd5e8
https://hg.mozilla.org/releases/mozilla-beta/rev/ac2e6a9da92c
status-firefox60:
--- → fixed
Assignee | ||
Updated•7 years ago
|
Status: RESOLVED → REOPENED
Flags: needinfo?(yzenevich)
Resolution: FIXED → ---
Assignee | ||
Comment 21•7 years ago
|
||
Attachment #8967484 -
Flags: review?(sdaswani)
Attachment #8967484 -
Flags: review?(sdaswani) → review+
Comment 22•7 years ago
|
||
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
Comment 23•7 years ago
|
||
bugherder |
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Updated•4 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
•