740696 - [Mac] we have children that are expired.

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect, P1)

Description

[Hubert Figuiere [:hub]]

We have children that are expired causing a ruckus in the hierarchy of accessible.

Comment 1

[Hubert Figuiere [:hub]]

Must fix.

Comment 2

[alexander :surkov (:asurkov)]

Ah, I told you we need to create normal tree on mac in bug 705404 and iirc pointed some cases when current approach won't work ;)

Comment 3

[Hubert Figuiere [:hub]]

One way to clearly see it is to start Firefox from the command line, pass -NSAccessibilityDebugLogLevel 1 as an argument and then start VoiceOver is needed. The console will print a lot of stuff.

Comment 4

[Hubert Figuiere [:hub]]

Attachment: Don't cache AXParent value. This basically revert bug 455443. r=

Comment 5

[Hubert Figuiere [:hub]]

Comment on attachment 612328 [details] [diff] [review]
Don't cache AXParent value. This basically revert bug 455443. r=

After thinking about it, this is IMHO the best course of action. Alex, what do you think? I basically revert the caching done for bug 455443.

Comment 6

[alexander :surkov (:asurkov)]

Comment on attachment 612328 [details] [diff] [review]
Don't cache AXParent value. This basically revert bug 455443. r=

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

you don't remove mParent member and this must hit the perf but in either case we need to rethink mac tree invalidation (that's what I permanently saying over months ;) ) so I'm fine with it as temporary solution so that you can continue to implement mac a11y support.

Comment 7

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #6)
> Comment on attachment 612328 [details] [diff] [review]
> Don't cache AXParent value. This basically revert bug 455443. r=
> 
> Review of attachment 612328 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> you don't remove mParent member and this must hit the perf but in either
> case we need to rethink mac tree invalidation (that's what I permanently
> saying over months ;) ) so I'm fine with it as temporary solution so that
> you can continue to implement mac a11y support.

well, tree update is an important part of that.

The patch seems correct mod the mParent member and whatever my head made me ignore.  However its not clear to me it would be better to actually fix this tree update stuffs finally rather than going in circles.

Comment 8

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #7)
> well, tree update is an important part of that.

having a real tree is important for performance, probably not for feature complete implementation

> The patch seems correct mod the mParent member and whatever my head made me
> ignore.  However its not clear to me it would be better to actually fix this
> tree update stuffs finally rather than going in circles.

that's my feeling I express over months. "best to loose a day learning ..." I'm saying but there's no unique way: http://www.youtube.com/watch?v=OZfPPLgCqTw :)

Comment 9

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #8)
> (In reply to Trevor Saunders (:tbsaunde) from comment #7)
> > well, tree update is an important part of that.
> 
> having a real tree is important for performance, probably not for feature
> complete implementation

yeah, but performance matters for users.

> > The patch seems correct mod the mParent member and whatever my head made me
> > ignore.  However its not clear to me it would be better to actually fix this
> > tree update stuffs finally rather than going in circles.
> 
> that's my feeling I express over months. "best to loose a day learning ..."
> I'm saying but there's no unique way:
> http://www.youtube.com/watch?v=OZfPPLgCqTw :)

sure, but its not good if all you do is fix x then fix y and then go back and undo x.

Comment 10

[Trevor Saunders (:tbsaunde)]

Comment on attachment 612328 [details] [diff] [review]
Don't cache AXParent value. This basically revert bug 455443. r=

well, why not.

Comment 11

[Hubert Figuiere [:hub]]

I definitely agree with trying to not go into circles and such.

And yes performance is also important.

Comment 12

[Hubert Figuiere [:hub]]

Landed inbound
https://hg.mozilla.org/integration/mozilla-inbound/

Comment 13

[Hubert Figuiere [:hub]]

I meant
https://hg.mozilla.org/integration/mozilla-inbound/rev/701495278c15

Comment 14

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/701495278c15