Closed Bug 1391733 Opened 2 years ago Closed 2 years ago

Update telemetry to provide information about e10s incompatible jaws usage

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: davidb, Assigned: davidb)

References

Details

(Whiteboard: aes+)

Attachments

(1 file, 1 obsolete file)

If we reuse the OLDJAWS slot of A11Y_CONSUMERS this might be a one liner here:
http://searchfox.org/mozilla-central/rev/e8c36327cd8c9432c69e5e1383156a74330f11f2/accessible/windows/msaa/Compatibility.cpp#142

(We anticipate Jaws <17 to be incompat)

This change will likely be useful for bug 1385991.
Whiteboard: aes+
Assignee: nobody → dbolter
Status: inquiring with VFO on Jaws version details to check here.
Jim this should work for us. The numbers are straight from VFO/Jaws. My build setup is currently broken, but I thought I'd post this untested since there is some urgency.
Attachment #8900259 - Flags: review?(jmathies)
Comment on attachment 8900259 [details] [diff] [review]
Morph unused OLDJAWS flag to identify e10s incompatible Jaws.

Review of attachment 8900259 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/windows/msaa/Compatibility.cpp
@@ +138,5 @@
>    // Note we collect some AT statistics/telemetry here for convenience.
>  
>    HMODULE jawsHandle = ::GetModuleHandleW(L"jhook");
>    if (jawsHandle)
> +    sConsumers |= (IsModuleVersionLessThan(jawsHandle, 18, 4315)) ?

OldJaws was used to disable IA2 support for builds prior 8 version. If you just bump the version up, then you disable IA2 support for all users of those versions.
Comment on attachment 8900259 [details] [diff] [review]
Morph unused OLDJAWS flag to identify e10s incompatible Jaws.

Ah right thank for the catch Alex!

Since we don't have any OLDJAWS users I think I'll remove the check. I doubt those old versions even work on supported Windows anymore.
Attachment #8900259 - Flags: review?(jmathies)
Yes, JAWS 7.10 is 12 years old now. This check is safe to remove.
Attached patch patch v2Splinter Review
As per chat we don't have any Jaws <=8 users so this is safe.
Attachment #8900259 - Attachment is obsolete: true
Attachment #8900721 - Flags: review?(surkov.alexander)
Comment on attachment 8900721 [details] [diff] [review]
patch v2

Review of attachment 8900721 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, taking into account comment #4 that we don't have any ODLJAWS users left, and assuming it's ok for telemetry that OLDJAWS metric will spike suddenly.
Attachment #8900721 - Flags: review?(surkov.alexander) → review+
(In reply to Marco Zehe (:MarcoZ) from comment #5)
> Yes, JAWS 7.10 is 12 years old now. This check is safe to remove.

no way, I cannot be that old! I do remember the users were crashing because of IA2 interfaces incompatibilities.
re: telemetry spiking, yeah, we're the only ones paying attention -- also I hope it doesn't spike ;)

I'm still struggling with my build setup, can someone shepherd this from here?
https://hg.mozilla.org/integration/mozilla-inbound/rev/9335b66895a9f6b298fd0beef679f9714b98344d
Bug 1391733 - Update telemetry to provide information about e10s incompatible jaws usage. r=surkov
Sorry all. Thanks for the quick recovery Alex!
(In reply to David Bolter [:davidb] (NeedInfo me for attention) from comment #12)
> Sorry all. Thanks for the quick recovery Alex!

I should never review the patches in a console on your laptop :)
https://hg.mozilla.org/mozilla-central/rev/9335b66895a9
https://hg.mozilla.org/mozilla-central/rev/bad6bb68e847
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Good news (so far).

Ratio of compat Jaws to incompat Jaws users is currently: [12,900:10]

(Note we're only currently detecting incompat on nightly/57)
Flags: needinfo?(jmathies)
(In reply to David Bolter [:davidb] (NeedInfo me for attention) from comment #15)
> Good news (so far).
> 
> Ratio of compat Jaws to incompat Jaws users is currently: [12,900:10]
> 
> (Note we're only currently detecting incompat on nightly/57)

I checked nightly data over on re:dash:

https://sql.telemetry.mozilla.org/queries/26633/source

I'd be willing to bet the 6 "new jaws" clients are all of us since technically we're the only people who have access to those newer builds.
Flags: needinfo?(jmathies) → needinfo?(dbolter)
Flags: needinfo?(dbolter)
Comment on attachment 8900721 [details] [diff] [review]
patch v2

Approval Request Comment
[Feature/Bug causing the regression]:
no bug, adding telemetry coverage for something
[User impact if declined]:
none
[Is this code covered by automated tests?]:
yes
[Has the fix been verified in Nightly?]:
yes
[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?]:
well understood code and change.
[String changes made/needed]:
none
Attachment #8900721 - Flags: approval-mozilla-beta?
(In reply to Ryan VanderMeulen [:RyanVM] from comment #14)
> https://hg.mozilla.org/mozilla-central/rev/9335b66895a9
> https://hg.mozilla.org/mozilla-central/rev/bad6bb68e847

There was a follow up fix. We'll need to land both patches.
Comment on attachment 8900721 [details] [diff] [review]
patch v2

Add telemetry for e10s incompatible jaws usage. Beta56+.
Attachment #8900721 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.