Closed Bug 1199735 Opened 4 years ago Closed 4 years ago

fire windows events on proxied accessibles

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: tbsaunde, Assigned: tbsaunde)

References

Details

Attachments

(3 files)

No description provided.
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.
Attachment #8654240 - Flags: review?(dbolter)
Attachment #8654242 - Flags: review?(dbolter)
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?
Attachment #8654240 - Flags: review?(dbolter) → review+
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...
Attachment #8654241 - Flags: review?(dbolter) → review+
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.
Attachment #8654242 - Flags: review?(dbolter) → review+
(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.
Assignee: nobody → tbsaunde+mozbugs
Blocks: e10sa11y2
You need to log in before you can comment on or make changes to this bug.