The DLBI patches broke invalidation of foreignObjects that are under an SVG filter, so it seems we don't have tests for that. Basically what nsSVGForeignObjectFrame::GetInvalidRegion does is wrong. It simply transforms the fO's bounds into nsSVGOuterSVGFrame space and invalidates that area, completely ignoring any filters along the parent chain. This seems to have happened because the nsSVGUtils::FindFilterInvalidation call in the original patch got changed to an nsSVGUtils::GetPostFilterVisualOverflowRect call after I killed off FindFilterInvalidation. Unfortunately, calling GetPostFilterVisualOverflowRect on the bounds transformed into nsSVGOuterSVGFrame space wasn't the right thing to replace it with though, since that skips filters along the way, only taking account of any filter that may be on the outer-<svg> itself. We should call nsSVGUtils::InvalidateBounds on the nsSVGForeignObjectFrame instead.
Created attachment 638136 [details] [diff] [review] patch
Attachment #638136 - Flags: review?(longsonr)
I'm leaving on vacation for a week in a few hours, so I'll probably not be able to land this. In fact it might be a good idea to wait and see if DLBI sticks before landing this so that if it needs to be backed out it doesn't add to the pain. Matt, if you do get backed out, feel free to just directly integrate the changes and test from this patch directly into your own patch, rather than committing it as a separate patch. (If that makes things easier.)
Can we get into InvalidateIfChildrenDirty if the foreignObject doesn't have any children? Do we have anything with a foreignObject that doesn't have children where we get an invalidation coming through?
The 'kid' in InvalidateIfChildrenDirty is the anonymous child that is always there to wrap any real children that may exist.
To be clear, the anonymous child is always there, whether or not there are any real descendants.
Attachment #638136 - Flags: review?(longsonr) → review+
Checked in the test as: https://hg.mozilla.org/integration/mozilla-inbound/rev/9aa4f137061e
Target Milestone: --- → mozilla17
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Should this bug have been closed?
Yes. Thanks, Ryan.
To be clear, the regression test is what Matt wanted checked in. He will/has just incorporated the fixes from the patch into his own patches as appropriate.
You need to log in before you can comment on or make changes to this bug.