1391733 - Update telemetry to provide information about e10s incompatible jaws usage

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[David Bolter [:davidb] (NeedInfo me for attention)]

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.

Comment 1

[David Bolter [:davidb] (NeedInfo me for attention)]

Status: inquiring with VFO on Jaws version details to check here.

Comment 2

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: Morph unused OLDJAWS flag to identify e10s incompatible Jaws.

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.

Comment 3

[alexander :surkov (:asurkov)]

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 4

[David Bolter [:davidb] (NeedInfo me for attention)]

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.

Comment 5

[Marco Zehe (:MarcoZ)]

Yes, JAWS 7.10 is 12 years old now. This check is safe to remove.

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: patch v2

As per chat we don't have any Jaws <=8 users so this is safe.

Comment 7

[alexander :surkov (:asurkov)]

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.

Comment 8

[alexander :surkov (:asurkov)]

(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.

Comment 9

[David Bolter [:davidb] (NeedInfo me for attention)]

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?

Comment 10

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9335b66895a9f6b298fd0beef679f9714b98344d
Bug 1391733 - Update telemetry to provide information about e10s incompatible jaws usage. r=surkov

Comment 11

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bad6bb68e8475aa1f910f1fe86539798b4ea138a
Bug 1391733 - get back missing IAccessible2 interface query, r=aklotz

Comment 12

[David Bolter [:davidb] (NeedInfo me for attention)]

Sorry all. Thanks for the quick recovery Alex!

Comment 13

[alexander :surkov (:asurkov)]

(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 :)

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9335b66895a9
https://hg.mozilla.org/mozilla-central/rev/bad6bb68e847

Comment 15

[David Bolter [:davidb] (NeedInfo me for attention)]

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)

Comment 16

[Jim Mathies [:jimm]]

(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.

Comment 17

[Jim Mathies [:jimm]]

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

Comment 18

[Jim Mathies [:jimm]]

(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 19

[Gerry Chang [:gchang]]

Comment on attachment 8900721 [details] [diff] [review]
patch v2

Add telemetry for e10s incompatible jaws usage. Beta56+.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a10406f37137

Comment 21

[David Bolter [:davidb] (NeedInfo me for attention)]

Jim, note this patch also explicitly de-supports Jaws versions prior to 18.4315. I don't think we want to do that in Beta right? I think we'll need a follow up for beta here that is more generous with handing out IID_IAccessible2.

Comment 22

[David Bolter [:davidb] (NeedInfo me for attention)]

Oh disregard that... We're good and I have coffee now.