Closed Bug 740696 Opened 8 years ago Closed 8 years ago

[Mac] we have children that are expired.

Categories

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

x86
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla14

People

(Reporter: hub, Assigned: hub)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

We have children that are expired causing a ruckus in the hierarchy of accessible.
Blocks: osxa11y
Must fix.
Assignee: nobody → hub
Priority: -- → P1
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 ;)
Blocks: 733513
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 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.
Attachment #612328 - Flags: feedback?(surkov.alexander)
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.
Attachment #612328 - Flags: review?(trev.saunders)
Attachment #612328 - Flags: feedback?(surkov.alexander)
Attachment #612328 - Flags: feedback+
(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.
(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 :)
(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 on attachment 612328 [details] [diff] [review]
Don't cache AXParent value. This basically revert bug 455443. r=

well, why not.
Attachment #612328 - Flags: review?(trev.saunders) → review+
I definitely agree with trying to not go into circles and such.

And yes performance is also important.
Landed inbound
https://hg.mozilla.org/integration/mozilla-inbound/
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/701495278c15
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.