There's no reason to check the object we just recreated isn't null, new is infalable. - use AsContent() instead of QueryInterface - don't use a weakFrame since the world will end if we modify the dom or layout while creating an accessible anyway.

Comment on attachment 666053 [details] [diff] [review] patch Review of attachment 666053 [details] [diff] [review]: ----------------------------------------------------------------- nice, thank you ::: accessible/src/base/nsAccessibilityService.cpp @@ +997,5 @@ > > roleMapEntry = nullptr; > } > > + if ( !newAcc && isHTML) { // HTML accessibles nit: excess space after ( @@ +998,5 @@ > roleMapEntry = nullptr; > } > > + if ( !newAcc && isHTML) { // HTML accessibles > + nsIAtom *frameType = frame->GetType(); nit: type* name @@ +1098,5 @@ > // the table caption would still be created. By setting > // *aIsSubtreeHidden = true we ensure that no descendant accessibles > // are created. > + if (frame->GetType() == nsGkAtoms::tableCaptionFrame && > + frame->GetRect().IsEmpty()) { nit: wrong indent of second line