Add in-process client information to Accessibility Instantiator support info

RESOLVED FIXED in Firefox 57

Status

()

RESOLVED FIXED
a year ago
a year ago

People

(Reporter: jimm, Assigned: jimm)

Tracking

Trunk
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: aes+)

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

a year ago
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
(Assignee)

Updated

a year ago
Depends on: 1385991
(Assignee)

Updated

a year ago
Blocks: 1392753
(Assignee)

Updated

a year ago
Blocks: 1394020
(Assignee)

Updated

a year ago
No longer depends on: 1385991
(Assignee)

Comment 1

a year ago
Created attachment 8902276 [details] [diff] [review]
patch
Assignee: nobody → jmathies
Attachment #8902276 - Flags: review?(eitan)
(Assignee)

Comment 2

a year ago
Created attachment 8902277 [details] [diff] [review]
patch v.2

- minor updates
Attachment #8902276 - Attachment is obsolete: true
Attachment #8902276 - Flags: review?(eitan)
Attachment #8902277 - Flags: review?(eitan)
(Assignee)

Comment 3

a year ago
Created attachment 8902288 [details] [diff] [review]
patch v.3

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

Comment 5

a year ago
Created attachment 8903762 [details] [diff] [review]
patch v.4
Attachment #8902288 - Attachment is obsolete: true
Attachment #8903762 - Flags: review?(eitan)
(Assignee)

Comment 6

a year ago
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+
(Assignee)

Comment 7

a year ago
Created attachment 8903836 [details] [diff] [review]
patch v.4
Attachment #8903762 - Attachment is obsolete: true
Attachment #8903836 - Flags: review+
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 8

a year ago
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
Last Resolved: a year ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.