1258670 - Stop using nsIDOMCustomEvent for TreeRowCountChanged / TreeInvalidated events

Status: NEW

(Core :: Disability Access APIs, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

nsTreeBodyFrame::FireRowCountChangedEvent
RootAccessible::HandleTreeRowCountChangedEvent
nsTreeBodyFrame::FireInvalidateEvent
RootAccessible::HandleTreeInvalidatedEvent

Not sure what the best solution is here: just add two new interfaces? I guess the code generation could handle them easily. NI for thoughts.

Comment 1

[alexander :surkov (:asurkov)]

In general, Gecko's core makes a11y notifications via nsAccessibilityService. I think those events be replaced on new methods in nsAccessibilityService class, similar to https://mxr.mozilla.org/mozilla-central/source/layout/xul/tree/nsTreeBodyFrame.cpp#508

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

surkov answered here.

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Stop firing TreeRowCountChanged / TreeInvalidated events

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

This doesn't dispatch async; will need more work if that's necessary.

Comment 5

[alexander :surkov (:asurkov)]

Comment on attachment 8733446 [details] [diff] [review]
Stop firing TreeRowCountChanged / TreeInvalidated events

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

r=me,thanks

::: accessible/base/nsAccessibilityService.cpp
@@ +646,5 @@
> +                                            int32_t aIndex,
> +                                            int32_t aCount)
> +{
> +  DocAccessible* targetDocument = GetDocAccessible(aContent->OwnerDoc());
> +  Accessible* accessible = targetDocument->GetAccessibleOrContainer(aContent);

GetBaseElement() returns a XUL tree element, you should be able to use targetDocument->GetAccessible(aContent)

@@ +667,5 @@
> +                                        int32_t aStartCol,
> +                                        int32_t aEndCol)
> +{
> +  DocAccessible* targetDocument = GetDocAccessible(aContent->OwnerDoc());
> +  Accessible* accessible = targetDocument->GetAccessibleOrContainer(aContent);

same here

::: layout/xul/tree/nsTreeBodyFrame.cpp
@@ +4726,5 @@
>  #ifdef ACCESSIBILITY
>  void
>  nsTreeBodyFrame::FireRowCountChangedEvent(int32_t aIndex, int32_t aCount)
>  {
> +  nsCOMPtr<nsIContent> content = GetBaseElement();

no nsCOMPtr is required

@@ +4742,5 @@
>  nsTreeBodyFrame::FireInvalidateEvent(int32_t aStartRowIdx, int32_t aEndRowIdx,
>                                       nsITreeColumn *aStartCol,
>                                       nsITreeColumn *aEndCol)
>  {
> +  nsCOMPtr<nsIContent> content = GetBaseElement();

same

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

I'm pretty sure you need to make it async. At least making this all working sync should happen in a different bug. async->sync changes are scary.

Comment 7

[alexander :surkov (:asurkov)]

(In reply to Olli Pettay [:smaug] from comment #6)
> I'm pretty sure you need to make it async. At least making this all working
> sync should happen in a different bug. async->sync changes are scary.

in general layout makes sync calls into a11y, so accService->TreeViewChanged does for example. We should be ok with sync calls there too, until I miss something

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

ok, if you think so fine. but super scary.