1199735 - fire windows events on proxied accessibles

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: remove event logging from the windows AccessibleWrap::HandleAccEvent

if this is useful it would make more sense to log it outside of the windows
layer.  Since its not clear it is useful, and it makes it harder to separate
event dispatch logic from HandleAccEvent its easiest to just remove it for now.

Comment 2

[Trevor Saunders (:tbsaunde)]

Attachment: factor win event dispatch logic into its own function

Comment 3

[Trevor Saunders (:tbsaunde)]

Attachment: fire windows events on proxies

Comment 4

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 8654240 [details] [diff] [review]
remove event logging from the windows AccessibleWrap::HandleAccEvent

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

r=me assuming surkov is ok with it?

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 8654241 [details] [diff] [review]
factor win event dispatch logic into its own function

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

::: accessible/windows/msaa/AccessibleWrap.cpp
@@ +1227,5 @@
> +  if (aEventType == nsIAccessibleEvent::EVENT_SELECTION &&
> +      Compatibility::IsJAWS()) {
> +    roles::Role role = aTarget->IsProxy() ? aTarget->Proxy()->Role() :
> +      aTarget->Role();
> +       if (role == roles::COMBOBOX_OPTION) {

Please remove 3 indent spaces in line above.

@@ +1334,5 @@
>      return nullptr;
>    }
>  
>    // Accessibles in child processes are said to have the HWND of the window
> +  // there tab is within.  Popups are always in the parent process, and so

nit: "there" should "their" (I know it was misspelled previously as well). I guess this these HWNDs are going away...

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 8654242 [details] [diff] [review]
fire windows events on proxies

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

LGTM, nits:

::: accessible/windows/msaa/Platform.cpp
@@ +80,3 @@
>  {
> +  AccessibleWrap* wrapper = WrapperFor(aTarget);
> +  AccessibleWrap::FireWinEvent(wrapper, aEventType);

Not sure what the variable gains us here.

@@ +87,4 @@
>  {
> +  AccessibleWrap* wrapper = WrapperFor(aTarget);
> +  AccessibleWrap::FireWinEvent(wrapper,
> +                               nsIAccessibleEvent::EVENT_STATE_CHANGE);

Not sure what the variable gains us here.

@@ +93,5 @@
>  void
>  a11y::ProxyCaretMoveEvent(ProxyAccessible* aTarget, int32_t aOffset)
>  {
> +  AccessibleWrap* wrapper = WrapperFor(aTarget);
> +  AccessibleWrap::FireWinEvent(wrapper, nsIAccessibleEvent::EVENT_TEXT_CARET_MOVED);

Not sure what the variable gains us here.

Comment 7

[alexander :surkov (:asurkov)]

(In reply to David Bolter [:davidb] from comment #4)
> Comment on attachment 8654240 [details] [diff] [review]
> remove event logging from the windows AccessibleWrap::HandleAccEvent
> 
> Review of attachment 8654240 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me assuming surkov is ok with it?

we chatted on irc on it. I used that in the past couple times, but I didn't for a while. Trevor convinced me the code can be reintroduced later at not big cost, if needed.

Comment 8

[David Bolter [:davidb] (NeedInfo me for attention)]

All good then :)

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/94f7fbe3876a
https://hg.mozilla.org/integration/mozilla-inbound/rev/f45bbef7719d
https://hg.mozilla.org/integration/mozilla-inbound/rev/078fc05c0a31

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/94f7fbe3876a
https://hg.mozilla.org/mozilla-central/rev/f45bbef7719d
https://hg.mozilla.org/mozilla-central/rev/078fc05c0a31