Closed Bug 1074917 Opened 5 years ago Closed 5 years ago

implemenet Accessible::State for proxied accessibles

Categories

(Core :: Disability Access APIs, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: tbsaunde, Assigned: tbsaunde)

References

Details

Attachments

(1 file)

No description provided.
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 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+
Attachment #8497572 - Flags: review?(mrbkap) → review+
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+
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
https://hg.mozilla.org/mozilla-central/rev/d6721fea9ad9
Assignee: nobody → tbsaunde+mozbugs
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Depends on: 1088148
You need to log in before you can comment on or make changes to this bug.