It was completely unclear to me from the name that nsSVGUtils::GetParentElement gets the flattened tree parent. I had to do a bit of digging before the change I was looking at made sense. How about calling it GetFlatTreeParent, and how about also moving it to nsIContent just after GetParent? That seems like a better name and location to me. If that's okay, this seems like a good first bug, and I have someone in mind. So no patches, okay. ;-)

Some kind of iterator class that allowed you to go up through the flat tree parents would be nice too. You would give it a starting element and it would call a virtual method for each ancestor till you told it to stop (return false from the virtual method) or it ran out of parents.

GetNearestViewport/GetFarthestViewport could be redone to use the iterator class.

I think we have an existing bug about having methods for iterating the flattened tree (needed for the DOM parts of XBL2), for what it's worth. Probably assigned to bryner or some such... I would probably be fine with putting something like this on nsIContent.

(In reply to comment #0) > It was completely unclear to me from the name that nsSVGUtils::GetParentElement > gets the flattened tree parent. I had to do a bit of digging before the change > I was looking at made sense. How about calling it GetFlatTreeParent, and how > about also moving it to nsIContent just after GetParent? That seems like a > better name and location to me. I just noticed that currently we already have nsIContent* nsIContent::GetFlattenedTreeParent() implemented in src/content/base/src/nsGenericElement.cpp but Element* nsSVGUtils::GetParentElement(nsIContent *aContent) has not been removed yet. So it seems that this bug has been partially fixed already. I am not sure if it is the correct way of doing it, but I am submitting a patch for review. I would like to get some feedback on it and in case it is wrong, I'd like to learn why.

You should pick a reviewer by entering an email address after you change the review flag to ?. In this case probably me or jwatt (http://www.mozilla.org/about/owners.html#svg).

Comment on attachment 472794 [details] [diff] [review] first try... Looks good - thanks!

Gah. Why didn't someone push this back in September? I'm going to take this temporarily so I remember to push it once Firefox 4 ships....

This bitrotted some time ago. It needs additional changes in nsSVGFilters.cpp at least now.

Like I said, "Gah". Felipe, do you want to update the patch yourself? If not, I'm happy to do that...

:/ Dunno why I didn't push it. Were we needing approval that far back? Anyways, I'm happy to update the patch since I'm mostly to blame here.

Felipe/bz: ping?

I'm planning to merge and push this; it just hasn't happened yet.

Attachment 524577 [details] [diff] is just the merge to tip. I pushed that as http://hg.mozilla.org/projects/cedar/rev/f6cb911d37da Reassigning back to Felipe, since this is his patch.

Thanks for the patch Felipe, and thanks for catching this Boris!