1694573 - Unify platform event firing across local and remote Accessibles

Status: RESOLVED FIXED

(Core :: Disability Access APIs, task)

Description

[James Teh [:Jamie]]

Currently, local and remote events are handled very differently. For local, we use AccEvent. For remote, we have a bunch of Proxy*Event methods. We want to be able to unify event handling for local and remote accessibles. To do this, AccEvent needs to hold an Accessible instead of a LocalAccessible.

Comment 1

[Morgan Reschenberg [:morgan]]

Attachment: Bug 1694573: Change AccEvent's mAccessible to use new Accessible base class. r?Jamie,eeejay

Comment 2

[James Teh [:Jamie]]

When we get to cleaning up events, I wonder whether we should flip this around and instead use (and rename) the Proxy*Event methods and use them for everything. That is, platforms would never deal with AccEvent directly. Advantages:

1. AccEvent won't contain a bunch of useless (and potentially confusing) members for remote events that are only useful for local events.
2. We don't have to deal with the ref counting stuff.
3. While we have AccessibleWrap, we have no equivalent for RemoteAccessible. We could move HandleAccEvent to Platform.h, but we already have these Proxy*Event functions, so we'd basically just end up calling the same functions or merging their code into one function.

To do this, we'd have LocalAccessible::HandleAccEvent call the new event firing functions as appropriate. The AccessibleWrap implementations could then be removed.

Eitan, thoughts?

Comment 3

[Eitan Isaacson [:eeejay]]

I think any unifying strategy is great. Thanks!

Comment 4

[James Teh [:Jamie]]

Eitan, it looks like we still use vc events for things like caret movement and scrolling from content to parent. I assume that's no longer strictly necessary? That is, the only thing we really need vc events for now is hit testing (because that still gets routed via content).

Comment 5

[James Teh [:Jamie]]

Having discussed this with Eitan, it looks like we can indeed clean up the Android VC event usage, but let's do this in a future bug.

Comment 6

[James Teh [:Jamie]]

Attachment: Bug 1694573 part 1: Rename Proxy*Event to Platform*Event.

This was done with the following command in the accessible/ directory:

sed -i 's/\bProxy\(.*\)Event\b/Platform\1Event/' `git grep -l 'Proxy.*Event'`

Comment 7

[James Teh [:Jamie]]

Attachment: Bug 1694573 part 2: Make Platform*Event functions take Accessible instead of RemoteAccessible.

Some of these methods don't yet handle LocalAccessible properly, but that will be fixed in subsequent patches.

Comment 8

[James Teh [:Jamie]]

Attachment: Bug 1694573 part 3: Add a unified version of Windows UpdateSystemCaretFor.

This takes an Accessible and a rect and calls the local or remote versions appropriately.
This avoids the duplication of conditional behaviour in PlatformFocusEvent and PlatformCaretMoveEvent.

Comment 9

[James Teh [:Jamie]]

Attachment: Bug 1694573 part 4: Make PlatformTextSelectionEvent take TextRange instead of TextRangeData.

TextRangeData is specific to IPDL and thus RemoteAccessible.

Comment 10

[James Teh [:Jamie]]

Attachment: Bug 1694573 part 5: Make LocalAccessible call Platform*Event. Remove most of the platform HandleAccEvent overrides.

Mac and Android still override HandleAccEvent for platform specific behaviour other than firing the event.
The Android behaviour can be unified properly in future work.

ATK is the platform layer with the most churn because there were inconsistencies in the way local and remote events were handled.
I'm reasonably sure these were unintentional inconsistencies, so I've done my best to unify them.

Comment 11

[James Teh [:Jamie]]

Attachment: Bug 1694573 part 6: Remove now empty Windows HyperTextAccessibleWrap.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/152a5f01d443
part 1: Rename Proxy*Event to Platform*Event. r=eeejay
https://hg.mozilla.org/integration/autoland/rev/060942e1d31f
part 2: Make Platform*Event functions take Accessible instead of RemoteAccessible. r=eeejay
https://hg.mozilla.org/integration/autoland/rev/56aae90925f8
part 3: Add a unified version of Windows UpdateSystemCaretFor. r=eeejay
https://hg.mozilla.org/integration/autoland/rev/54d3449fca3c
part 4: Make PlatformTextSelectionEvent take TextRange instead of TextRangeData. r=eeejay
https://hg.mozilla.org/integration/autoland/rev/b59caf87b3b2
part 5: Make LocalAccessible call Platform*Event. Remove most of the platform HandleAccEvent overrides. r=eeejay
https://hg.mozilla.org/integration/autoland/rev/2f270a08993b
part 6: Remove now empty Windows HyperTextAccessibleWrap. r=eeejay
https://hg.mozilla.org/integration/autoland/rev/4f35a86f6d3c
412902: apply code formatting via Lando

Comment 13

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/152a5f01d443
https://hg.mozilla.org/mozilla-central/rev/060942e1d31f
https://hg.mozilla.org/mozilla-central/rev/56aae90925f8
https://hg.mozilla.org/mozilla-central/rev/54d3449fca3c
https://hg.mozilla.org/mozilla-central/rev/b59caf87b3b2
https://hg.mozilla.org/mozilla-central/rev/2f270a08993b
https://hg.mozilla.org/mozilla-central/rev/4f35a86f6d3c