728905 - telemetry ZoomText, YouDao Dictionary and Kazaguru

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

ZoomText dll is WebFinderRemote.dll

1) Add new constant for this dll (http://mxr.mozilla.org/mozilla-central/source/accessible/src/msaa/Compatibility.h#103)
1) Check new consumer in Compatibility::Init()
2) Fix TelemetryHistograms.h (http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetryHistograms.h#59)

Trevor, would be nice if you can steal mentoring.

Comment 1

[alexander :surkov (:asurkov)]

making Trevor a mentor per irc

Comment 2

[Mark Capella [:capella]]

At first glance, it looks like we add / change the following code snips:

compatibility.h:

   /**
    * List of detected consumers of a11y (used for statistics/telemetry)
    */
   enum {
     NVDA = 0,
     JAWS = 1,
     OLDJAWS = 2,
     WE = 3,
     DOLPHIN = 4,
     SEROTEK = 5,
     COBRA = 6,
     ZOOMHOOK = 7
   };

telemetryhistograms.h:

   HISTOGRAM(A11Y_CONSUMERS, 1, 7, 8, LINEAR, "Accessibility client by enum id")

compatibility.cpp

  if (::GetModuleHandleW(L"WebFinderRemote"))
    statistics::A11yConsumers(ZOOMTEXT);


The last part is where my questions are... I'm assuming no special sMode, nor new tab switching code as with JAWS / WE / DOLPHIN. Also assuming that the handle is the name of the DLL.

How is the packaging of the code done, that is, where do the DLL's reside? Local to the client or remote at MOZILLA or DLL provider perhaps?

Comment 3

[Mark Capella [:capella]]

Added mentor to the cc list ...

Comment 4

[alexander :surkov (:asurkov)]

(In reply to Mark Capella [:capella] from comment #2)

> The last part is where my questions are... I'm assuming no special sMode,
> nor new tab switching code as with JAWS / WE / DOLPHIN.

yes

> Also assuming that
> the handle is the name of the DLL.

not sure what you mean
 
> How is the packaging of the code done, that is, where do the DLL's reside?
> Local to the client or remote at MOZILLA or DLL provider perhaps?

locally, this dll is a part of 3d party application - zoom text (http://www.aisquared.com/zoomtext). If the user installed it and running it together with Firefox then this code triggers and we collect this data.

Comment 5

[Trevor Saunders (:tbsaunde)]

> > Also assuming that
> > the handle is the name of the DLL.
> 
> not sure what you mean

I think he wanted to make sure the dll's name was the thing to pass to GetModuleHandleW() which I believe is correct.

Comment 6

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #5)
> > > Also assuming that
> > > the handle is the name of the DLL.
> > 
> > not sure what you mean
> 
> I think he wanted to make sure the dll's name was the thing to pass to
> GetModuleHandleW() which I believe is correct.

Ah, yes, that's right. GetModuleHandle takes dll file name.

Comment 7

[Mark Capella [:capella]]

Attachment: Patch (v1)

Code changes effect a clean local clobber build. 

How to test soon becomes relevent ...

Comment 8

[Trevor Saunders (:tbsaunde)]

Comment on attachment 600660 [details] [diff] [review]
Patch (v1)

canceling review for bug 730198 comment 3

Comment 9

[Mark Capella [:capella]]

Attachment: Patch (v2)

Rolled patch for bug73198 and bug728905 into this one ...

Comment 10

[Mark Capella [:capella]]

Sorry ... rolled in bug730198

Comment 12

[alexander :surkov (:asurkov)]

Comment on attachment 600671 [details] [diff] [review]
Patch (v2)

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

f=me with comment addressed

::: accessible/src/msaa/Compatibility.h
@@ +109,5 @@
>      SEROTEK = 5,
> +    COBRA = 6,
> +    ZOOMTEXT = 7,
> +    KAZAHOOK = 8,
> +    TEXTEXTRACTOR = 9

I'd prefer application names, that's better for readability

Comment 13

[Mark Capella [:capella]]

Attachment: Patch (v3)

Comment 14

[alexander :surkov (:asurkov)]

Comment on attachment 600683 [details] [diff] [review]
Patch (v3)

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

::: accessible/src/msaa/Compatibility.cpp
@@ +123,5 @@
> +  if (::GetModuleHandleW(L"Kazahook"))
> +    statistics::A11yConsumers(KAZAGURU);
> +
> +  if ((::GetModuleHandleW(L"TextExtractorImpl32")) ||
> +      (::GetModuleHandleW(L"TextExtractorImpl64")))

I'm curious why do you wrap the method call by braces, otherwise is fine, f+, again :)

Comment 15

[Mark Capella [:capella]]

Oh.... hmmm... cut 'n paste overkill  :-(

Do I need to re-ask for review+ to move forward? Since I don't have L1 I usually have to get a review+ then I can AUTOLAND to TRY, when that passes I can CHECKIN-NEEDED ... 

(On our last bug728904 together, you did alot of this for me.)

Comment 16

[alexander :surkov (:asurkov)]

I'll land it, don't worry. I do that for a11y patches.

Comment 17

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a32cc7faa71a

Comment 18

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/a32cc7faa71a