Bug 772346 comment #8:
> This code is probably wrong anyway, for the same reason as in bug 767684.
> It now says
>
> for (nsCOMPtr<nsIContent> child = aNode->GetLastChild();
> child;
> child = child->GetPreviousSibling()) {
> nsresult rv = DeleteNonTableElements(child);
> NS_ENSURE_SUCCESS(rv, rv);
> }
>
> But DeleteNonTableElements(child) might remove child, so GetPreviousSibling
> will incorrectly return null. Changing it back to the way it was should
> both fix the use-after-free and make it correctly affect all children:
>
> for (PRInt32 i = aNode->GetChildCount() - 1; i >= 0; --i) {
> nsresult rv = DeleteNonTableElements(aNode->GetChildAt(i));
> NS_ENSURE_SUCCESS(rv, rv);
> }
I attached a patch to bug 772332, but this should really have its own bug, particularly since the error has now crept into Aurora.
Ehsan, you asked for a backout, but the patch no longer backs out cleanly. E.g., it needs changes for bug 763283 part 2, and for bug 772807. Should we just backport the patch from bug 772332 to Aurora instead?

(In reply to :Aryeh Gregor from comment #0)
> Ehsan, you asked for a backout, but the patch no longer backs out cleanly.
> E.g., it needs changes for bug 763283 part 2, and for bug 772807. Should we
> just backport the patch from bug 772332 to Aurora instead?
Sure.

Created attachment 644251[details][diff][review]
Patch (already reviewed by ehsan on bug 772332)
[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 755264
User impact if declined: Unknown -- no test case was produced that actually exhibits a problem. The incorrect change caused bug 772346 (use-after-free), which was fixed separately, but no specific test-case was given. However, the same erroneous change was made in a different place by bug 752210, which caused bug 767684. It's likely that there are some cases where this bug breaks editor actions, where the user tries to delete some content but fewer elements are deleted than expected.
Testing completed (on m-c, etc.): None yet. The patch reverts to the same logic as was in place before bug 755264, however, so in that sense the code has years of testing.
Risk to taking this patch (and alternatives if risky): Low. The patch just reverts a logic change to the way it was before, and the change is very simple. It's hard to see how it could cause problems.
String or UUID changes made by this patch: None.
I'll check the patch into m-i when it opens. (The patch was already pushed in bug 772332 comment 10, but the push was backed out due to a problem with another patch in the same push, so it didn't make it to 16 before the uplift.)