Closed
Bug 1074917
Opened 10 years ago
Closed 10 years ago
implemenet Accessible::State for proxied accessibles
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: tbsaunde, Assigned: tbsaunde)
References
Details
Attachments
(1 file)
No description provided.
Assignee | ||
Comment 1•10 years ago
|
||
We need to implement things like
https://developer.gnome.org/atk/unstable/AtkObject.html#atk-object-ref-state-set
and the same basic thing on windows. That API is fundimentally sync,
but the information necessary to implement it is only available in the
child process. That seems to leave us with two options, either we can
use sync ipc or we can use async ipc but spin a nested event loop. If
we were to spin neested event loops we'd have to be careful to make sure
a11y didn't do anything until the nested event loop was done, and then
a11y would have to deal with whatever changed. I'm not sure that will
work, and since the system is probably waiting for the accessibility
information anyway I don't think we get much out of spinning the event
loop. So I think its somewhat less bad to use sync ipc here.
Attachment #8497572 -
Flags: review?(surkov.alexander)
Attachment #8497572 -
Flags: review?(mrbkap)
Comment 2•10 years ago
|
||
Comment on attachment 8497572 [details] [diff] [review]
teach atk to get states from proxies
Review of attachment 8497572 [details] [diff] [review]:
-----------------------------------------------------------------
I don't see problems but it'd be good if David looked since he follows this work close
::: accessible/atk/AccessibleWrap.cpp
@@ +899,5 @@
> + if (accWrap)
> + TranslateStates(accWrap->State(), state_set);
> + else if (ProxyAccessible* proxy = GetProxy(aAtkObj))
> + TranslateStates(proxy->State(), state_set);
> + else
nit: wrong indentation
I'm curious whether we can avoid copy/paste code, that doesn't look good, especially in bug 1074869.
Attachment #8497572 -
Flags: review?(surkov.alexander)
Attachment #8497572 -
Flags: review?(dbolter)
Attachment #8497572 -
Flags: review+
Updated•10 years ago
|
Attachment #8497572 -
Flags: review?(mrbkap) → review+
Comment 3•10 years ago
|
||
Comment on attachment 8497572 [details] [diff] [review]
teach atk to get states from proxies
Review of attachment 8497572 [details] [diff] [review]:
-----------------------------------------------------------------
OK. I expect we'll revisit the design (e.g. cache more) as we measure real world performance. A couple optional nits in your commit message: 'fundimentally' should be 'fundamentally' and 'neested' should give up an e.
::: accessible/atk/AccessibleWrap.cpp
@@ +899,5 @@
> + if (accWrap)
> + TranslateStates(accWrap->State(), state_set);
> + else if (ProxyAccessible* proxy = GetProxy(aAtkObj))
> + TranslateStates(proxy->State(), state_set);
> + else
I wish we'd always use {}'s.
::: accessible/ipc/PDocAccessible.ipdl
@@ +39,5 @@
> ShowEvent(ShowEventData data);
> HideEvent(uint64_t aRootID);
> +
> +child:
> + rpc State(uint64_t aID) returns(uint64_t states);
ok
::: accessible/ipc/ProxyAccessible.cpp
@@ +42,5 @@
> +uint64_t
> +ProxyAccessible::State() const
> +{
> + uint64_t state = 0;
> + unused << mDoc->CallState(mID, &state);
TIL unused.
Attachment #8497572 -
Flags: review?(dbolter) → review+
Comment 4•10 years ago
|
||
backed this out in https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-inbound&revision=49b06fb31f0b together with the other landings from that push (and bug 1074917 which caused conflicts during backout) for the regression mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=982842#c41
Assignee: nobody → tbsaunde+mozbugs
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•