Closed Bug 1393987 Opened 7 years ago Closed 7 years ago

Add in-process client information to Accessibility Instantiator support info

Categories

(Core :: Disability Access APIs, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: jimm, Assigned: jimm)

References

Details

(Whiteboard: aes+)

Attachments

(1 file, 4 obsolete files)

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
Depends on: 1385991
Blocks: 1392753
Blocks: 1394020
No longer depends on: 1385991
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → jmathies
Attachment #8902276 - Flags: review?(eitan)
Attached patch patch v.2 (obsolete) — Splinter Review
- minor updates
Attachment #8902276 - Attachment is obsolete: true
Attachment #8902276 - Flags: review?(eitan)
Attachment #8902277 - Flags: review?(eitan)
Attached patch patch v.3 (obsolete) — Splinter Review
One more rev. I realized sConsumers will always have a value, so I added a GetAccService() check.
Attachment #8902277 - Attachment is obsolete: true
Attachment #8902277 - Flags: review?(eitan)
Attachment #8902288 - Flags: review?(eitan)
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.
Attachment #8902288 - Flags: review?(eitan)
Attached patch patch v.4 (obsolete) — Splinter Review
Attachment #8902288 - Attachment is obsolete: true
Attachment #8903762 - Flags: review?(eitan)
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.
Attachment #8903762 - Flags: review?(eitan) → review+
Attached patch patch v.4Splinter Review
Attachment #8903762 - Attachment is obsolete: true
Attachment #8903836 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3bfaec8923dd
Add in-process client information to Accessibility Instantiator support info. r=eeejay
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/3bfaec8923dd
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.