614552 - fire reorder event on XUL tree when treeview is changed

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: patch

Now we fire platform show/hide events on XUL tree when tree view is changed. The reality is show/hide events aren't used by AT in general. But they listen for reorder events. It's more expected to use it instead of platform show/hide events. Ideally we should fire hide events for all old tree items and new for new tree items but I afraid of perf issue. Therefore I suggest to fire reorder event only and leave show/hide events issue for future.

Comment 1

[alexander :surkov (:asurkov)]

I need this for bug 498015 where I'm going to process DOM events after layout. So that mochitest can't rely on TreeViewChanged DOM event anymore.

Comment 2

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

I think we need a try build here.

Comment 3

[alexander :surkov (:asurkov)]

np - http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-6a0fefcfd139

Comment 4

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

Comment on attachment 492988 [details] [diff] [review]
patch

Removing patch approval request as this blocks a blocker now.

Comment 5

[Marco Zehe (:MarcoZ)]

Comment on attachment 492988 [details] [diff] [review]
patch

The patch looks straight-forward, but I'd like to see a try build before I can r+ this one.

Comment 6

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

Comment on attachment 492988 [details] [diff] [review]
patch

>@@ -591,36 +591,28 @@ nsXULTreeAccessible::TreeViewInvalidated

> void
> nsXULTreeAccessible::TreeViewChanged()

>-
>   mTree->GetView(getter_AddRefs(mTreeView));

What is this for?

Comment 7

[alexander :surkov (:asurkov)]

(In reply to comment #6)

> >-
> >   mTree->GetView(getter_AddRefs(mTreeView));
> 
> What is this for?

line removal or mTreeView intializing when tree view was changed?

Comment 8

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

The latter. This call initializes the tree view?

Comment 9

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

Comment on attachment 492988 [details] [diff] [review]
patch

r=me, ignore my last comment/question (I totally missed the mTreeView somehow). Not this r+ is conditional on not breaking screen readers ;)

Comment 10

[Marco Zehe (:MarcoZ)]

Comment on attachment 492988 [details] [diff] [review]
patch

Yep this works, thanks! r=me.

Comment 11

[alexander :surkov (:asurkov)]

landed on 2.0 - http://hg.mozilla.org/mozilla-central/rev/6038dc8945d8