spun off bug 759875. DocAccessible::IsInDocument returns wrong result for an accessible that was destroyed and new accessible for the same DOM node was created. 1) add eIsNotInDocument flag to Accessible::StateFlags (don't forget to update AccessibleTypes enum) 2) add IsInDocument() method to Accessible 3) remove DocAccessible::IsInDocument 4) make DocAccessible::UncacheChildrenInSubtree to set IsNotInDocument flag on the accessible

I followed what I thought the intent was here, but I'm getting test failures in test_stale.html. Did I mess up the code? Or simply expose the problem we're trying to fix? Ties into original Bug 676267 ... ?

I think because of inline bool IsInDocument() const { return mFlags & eIsNotInDocument; }

and because of this DocAccessible::UncacheChildrenInSubtree(Accessible* aRoot) { mFlags |= eIsNotInDocument;

Lord, I've got to stop jumping on bugs I don't fully understand :) This seems to make more sense, and it passes all tests ...

Comment on attachment 631831 [details] [diff] [review] Patch (v2) Review of attachment 631831 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/src/generic/Accessible.h @@ +479,5 @@ > return mContent->IsHTML() && > (mContent->Tag() == nsGkAtoms::abbr || mContent->Tag() == nsGkAtoms::acronym); > } > > + inline bool IsInDocument(Accessible* aAccessible) const no inline, I'd keep it near of IsDefunct, please comment it @@ +484,5 @@ > + { > + Accessible* acc = aAccessible; > + while (acc && !acc->IsPrimaryForNode()) > + acc = acc->Parent(); > + you don't need that because you set the flag on each accessible you uncache from document @@ +488,5 @@ > + > + return acc ? !(acc->mFlags & eIsNotInDocument) : false; > + } > + > + inline void SetIsNotInDocument() { mFlags |= eIsNotInDocument; } not sure I like it's public (you could friend classes) but perhaps it's ok.

I went ahead and tweaked those bits while waiting ...

Comment on attachment 631904 [details] [diff] [review] Patch (v3) Review of attachment 631904 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/src/generic/Accessible.h @@ +694,5 @@ > + */ > + bool IsInDocument(Accessible* aAccessible) const > + { > + return aAccessible ? !(aAccessible->mFlags & eIsNotInDocument) : false; > + } you're on accessible so you don't need aAccessible argument, jsut return !(mFlags & eIsNotInDocument);

I though I tried that and it was failinng ... lemme try again ...

Comment on attachment 631904 [details] [diff] [review] Patch (v3) ># HG changeset patch ># Parent 0f6ae110182a267ab1a20bca6730194cbff72ea3 ># User Mark Capella <markcapella@twcny.rr.com> >Bug 760354 - implement IsInDocument as accessible flag, r=surkov, f=tbsaunde might want to update that ;-) >+ bool IsInDocument(Accessible* aAccessible) const >+ { >+ return aAccessible ? !(aAccessible->mFlags & eIsNotInDocument) : false; if we're going to move this from DocAccessible to Accessible we should get rid of the argument and make it work on this, and then update the callers. Hopefully that'll help perf some since callers won't need to get the doc anymore.

This is how I had the patch without passing an argument ... it fails test_stale.html with: failed | wrong extra state bits for ['div node', address: [object HTMLDivElement], role: section, address: [xpconnect wrapped nsIAccessible]]!got '0', expected 'stale' Changing it to take the arg works / passes ... (I had the debugger out and was hacking at it for awhile before I tried the new approach).

Changed the caller and this works again ...

push to inbound TRY https://tbpl.mozilla.org/?tree=Try&rev=314aed2afe92

previous push was in the middle of many TRY and INBOUND issues ... lotsa backouts and such ... waited, and am trying again: https://tbpl.mozilla.org/?tree=Try&rev=982d37b69ab6