Closed
Bug 1074869
Opened 10 years ago
Closed 10 years ago
Make atk deal with proxied focus events
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: tbsaunde, Assigned: tbsaunde)
Details
Attachments
(2 files)
5.33 KB,
patch
|
davidb
:
review+
|
Details | Diff | Splinter Review |
7.91 KB,
patch
|
davidb
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8497534 -
Flags: review?(dbolter)
Updated•10 years ago
|
Summary: Make deal with proxied focus events → Make atk deal with proxied focus events
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.
Attachment #8497534 -
Flags: review?(dbolter) → review+
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
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8519306 -
Flags: review?(dbolter)
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?
Attachment #8519306 -
Flags: review?(dbolter) → review+
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.
Assignee | ||
Comment 7•10 years ago
|
||
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
•