769942 - DLBI broke invalidation of foreignObjects in filters

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jonathan Watt [:jwatt]]

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.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 2

[Jonathan Watt [:jwatt]]

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.)

Comment 3

[Robert Longson [:longsonr]]

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?

Comment 4

[Jonathan Watt [:jwatt]]

The 'kid' in InvalidateIfChildrenDirty is the anonymous child that is always there to wrap any real children that may exist.

Comment 5

[Jonathan Watt [:jwatt]]

To be clear, the anonymous child is always there, whether or not there are any real descendants.

Comment 6

[Jonathan Watt [:jwatt]]

Checked in the test as:

https://hg.mozilla.org/integration/mozilla-inbound/rev/9aa4f137061e

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/9aa4f137061e

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Should this bug have been closed?

Comment 9

[Jonathan Watt [:jwatt]]

Yes. Thanks, Ryan.

Comment 10

[Jonathan Watt [:jwatt]]

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.