Trevor Saunders (:tbsaunde) Assignee
	Description • 10 years ago

I imagine at some point we may need to do something smarter because of multiple windows and what not, but it seems to me this patch is better than nothing.

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 2 • 10 years ago

Comment on attachment 8497534 [details] [diff] [review]
make atk deal with proxied focus events

Review of attachment 8497534 [details] [diff] [review]:
-----------------------------------------------------------------

ok
(note to readers: requires bug 982842)

::: accessible/ipc/DocAccessibleParent.cpp
@@ +118,5 @@
> +    return true;
> +  }
> +
> +  ProxyAccessible* proxy = e->mProxy;
> +  ProxyEvent(proxy, aEventType);

Should we assert for proxy here? I think ProxyEvent fails silently otherwise. Not sure how I feel about that.

	Trevor Saunders (:tbsaunde) Assignee
	Comment 3 • 10 years ago

(In reply to David Bolter [:davidb] from comment #2)
> Comment on attachment 8497534 [details] [diff] [review]
> make atk deal with proxied focus events
> 
> Review of attachment 8497534 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ok
> (note to readers: requires bug 982842)
> 
> ::: accessible/ipc/DocAccessibleParent.cpp
> @@ +118,5 @@
> > +    return true;
> > +  }
> > +
> > +  ProxyAccessible* proxy = e->mProxy;
> > +  ProxyEvent(proxy, aEventType);
> 
> Should we assert for proxy here? I think ProxyEvent fails silently
> otherwise. Not sure how I feel about that.

probably, though I'm not really sure why we need to fail silently, its kind of coppied from AccessibleWrap::HandleEvent

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 5 • 10 years ago

Comment on attachment 8519306 [details] [diff] [review]
make atk deal with proxied focus events

Review of attachment 8519306 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/atk/AccessibleWrap.cpp
@@ +1309,5 @@
>  
> +void
> +a11y::ProxyEvent(ProxyAccessible* aTarget, uint32_t aEventType)
> +{
> +  AtkObject* wrapper = GetWrapperFor(aTarget);

Why did you remove the !wrapper check?

::: accessible/base/EventQueue.cpp
@@ +481,5 @@
>  }
>  
> +void
> +EventQueue::SendIPCEvent(AccEvent* aEvent) const
> +{

Good idea.

@@ +497,5 @@
> +      break;
> +
> +    case nsIAccessibleEvent::EVENT_REORDER:
> +      // reorder events on the application acc aren't necessary to tell the parent
> +      // about new top level documents.

I'm curious what happens if we do?

	Trevor Saunders (:tbsaunde) Assignee
	Comment 6 • 10 years ago

> ::: accessible/atk/AccessibleWrap.cpp
> @@ +1309,5 @@
> >  
> > +void
> > +a11y::ProxyEvent(ProxyAccessible* aTarget, uint32_t aEventType)
> > +{
> > +  AtkObject* wrapper = GetWrapperFor(aTarget);
> 
> Why did you remove the !wrapper check?

I don't see how it can possibly be true

> @@ +497,5 @@
> > +      break;
> > +
> > +    case nsIAccessibleEvent::EVENT_REORDER:
> > +      // reorder events on the application acc aren't necessary to tell the parent
> > +      // about new top level documents.
> 
> I'm curious what happens if we do?

we get an assertion because the parent process doesn't know about the accessible for the childs Application acc.

	Trevor Saunders (:tbsaunde) Assignee
	Comment 7 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/f991f366f66e

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 8 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/f991f366f66e