1393987 - Add in-process client information to Accessibility Instantiator support info

Status: RESOLVED FIXED

(Core :: Disability Access APIs, enhancement)

Description

[Jim Mathies [:jimm]]

We currently display the exe name of detected oop clients.. we should display in-process client info as well. I'd suggest we simply update the code for GetAccessibilityInstantiator [1] to display the exe and the compatibility flags as a hex value.

[1] http://searchfox.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#981

Comment 1

[Jim Mathies [:jimm]]

Attachment: patch

Comment 2

[Jim Mathies [:jimm]]

Attachment: patch v.2

- minor updates

Comment 3

[Jim Mathies [:jimm]]

Attachment: patch v.3

One more rev. I realized sConsumers will always have a value, so I added a GetAccService() check.

Comment 4

[Eitan Isaacson [:eeejay]]

Comment on attachment 8902288 [details] [diff] [review]
patch v.3

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

Just a couple of suggestions below.

::: toolkit/xre/nsAppRunner.cpp
@@ +985,5 @@
>      aInstantiator = NS_LITERAL_STRING("");
> +    return NS_OK;
> +  }
> +  nsAutoString oopClientInfo, ipClientInfo;
> +  ipClientInfo.AppendPrintf("0x%X", mozilla::a11y::Compatibility::sConsumers);

Wouldn't it be nicer if we had a human readable string? We can add a consumer map with constant strings and do something like:

uint32_t index = 0;
for (uint32_t consumers = sConsumers; consumers; consumers = consumers >> 1) {
  if (consumers & 1)
    ipClientInfo.Append(ConsumerMap[index]);
  index++
}

@@ +990,5 @@
> +  a11y::GetInstantiator(oopClientInfo);
> +  aInstantiator.Append(ipClientInfo);
> +  aInstantiator.AppendLiteral("|");
> +  if (!oopClientInfo.IsEmpty()) {
> +    aInstantiator.Append(oopClientInfo);

Why check if it is empty here? It would be more useful to check before appending the pipe delimiter above.

Comment 5

[Jim Mathies [:jimm]]

Attachment: patch v.4

Comment 6

[Jim Mathies [:jimm]]

Comment on attachment 8903762 [details] [diff] [review]
patch v.4

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

::: accessible/windows/msaa/Compatibility.h
@@ +85,2 @@
>  
> +public:

this change isn't needed, removed locally.

Comment 7

[Jim Mathies [:jimm]]

Attachment: patch v.4

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3bfaec8923dd
Add in-process client information to Accessibility Instantiator support info. r=eeejay

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/3bfaec8923dd