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)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: jimm, Assigned: jimm)
References
Details
(Whiteboard: aes+)
Attachments
(1 file, 4 obsolete files)
4.51 KB,
patch
|
jimm
:
review+
|
Details | Diff | Splinter Review |
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 | ||
Comment 1•7 years ago
|
||
Assignee: nobody → jmathies
Attachment #8902276 -
Flags: review?(eitan)
Assignee | ||
Comment 2•7 years ago
|
||
- minor updates
Attachment #8902276 -
Attachment is obsolete: true
Attachment #8902276 -
Flags: review?(eitan)
Attachment #8902277 -
Flags: review?(eitan)
Assignee | ||
Comment 3•7 years ago
|
||
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 4•7 years ago
|
||
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•7 years ago
|
||
Attachment #8902288 -
Attachment is obsolete: true
Attachment #8903762 -
Flags: review?(eitan)
Assignee | ||
Comment 6•7 years 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.
Updated•7 years ago
|
Attachment #8903762 -
Flags: review?(eitan) → review+
Assignee | ||
Comment 7•7 years ago
|
||
Attachment #8903762 -
Attachment is obsolete: true
Attachment #8903836 -
Flags: review+
Assignee | ||
Updated•7 years ago
|
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
Comment 9•7 years ago
|
||
bugherder |
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.
Description
•