spun off bug 638326. I think it should be fixed by bug 634202. But prefer to keep this bug separately just in case. Don't forget to fix todo in text/test_hypertext.html.

Comment on attachment 8375709 [details] [diff] [review] patch >+ if (childNode == anchorNode) >+ return NextChildInternal(false); >+ } >+ PopState(); >+ >+ anchorNode = parentNode->AsElement(); why do you need the AsElement() >+ enum { >+ eWalkCache = 1, >+ eWalkContextTree = 2 | eWalkCache Its weird how one flag implies another. also comment what they mean? > virtual ~TreeWalker(); btw why is that virtual? >+ descendant = mDoc->GetAccessible(findNode); >+ if (!descendant && findNode->IsElement()) { >+ Accessible* container = mDoc->GetContainerAccessible(findNode); what's wrong with text nodes?

(In reply to Trevor Saunders (:tbsaunde) from comment #3) > >+ anchorNode = parentNode->AsElement(); > > why do you need the AsElement() to assign it to anchorNode which is a content > >+ enum { > >+ eWalkCache = 1, > >+ eWalkContextTree = 2 | eWalkCache > > Its weird how one flag implies another. also comment what they mean? not sure how to make it nicer, I don't want to have an option to go outside the given element and create the tree. I will document. > > virtual ~TreeWalker(); > > btw why is that virtual? change it to MOZ_FINAL while I'm here? > >+ descendant = mDoc->GetAccessible(findNode); > >+ if (!descendant && findNode->IsElement()) { > >+ Accessible* container = mDoc->GetContainerAccessible(findNode); > > what's wrong with text nodes? text nodes should be accessible in general, I guess it's technically possible to stop at not accessible text node though. I will change that to nsIContent then.

> > >+ enum { > > >+ eWalkCache = 1, > > >+ eWalkContextTree = 2 | eWalkCache > > > > Its weird how one flag implies another. also comment what they mean? > > not sure how to make it nicer, I don't want to have an option to go outside > the given element and create the tree. I will document. fair enough > > > virtual ~TreeWalker(); > > > > btw why is that virtual? > > change it to MOZ_FINAL while I'm here? it shouldn't have any virtual functions, but I guess you might as well.