Closed Bug 1494604 Opened 2 years ago Closed 2 years ago

Figure out what to do with nsSVGUtils::NotifyAncestorsOfFilterRegionChange

Categories

(Core :: SVG, enhancement, P3)

Product:
Core
Component:
SVG
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla64
Tracking Flags:
Tracking Status
firefox64 --- fixed

People

(Reporter: jwatt, Assigned: jwatt)

Details

Attachments

(1 file)

Bug 1494604. Remove nsSVGUtils::NotifyAncestorsOfFilterRegionChange. r?longsonr
46 bytes, text/x-phabricator-request
longsonr
: review+
Details | Review 
nsSVGUtils::NotifyAncestorsOfFilterRegionChange simply walks the (SVG-only) parent chain, essentially dispatching restyle events with nsChangeHint_UpdateOverflow for each ancestor.

However, the only caller of this function is nsSVGDisplayContainerFrame::RemoveFrame, and it already dispatches an nsChangeHint_UpdateOverflow for the frame that's being removed (and overflow changes should bubble up through the ancestors when they're calculated). Maybe that gets discarded if the element itself is being removed? In any case, surely then we'd just need to dispatch nsChangeHint_UpdateOverflow for the parent frame, not every ancestor.

If the NotifyAncestorsOfFilterRegionChange isn't doing anything useful, we should remove it. If it is necessary, we should comment why.
The NotifyAncestorsOfFilterRegionChange code was originally added in bug 421584. Since then we seem to have removed all other consumers except for the call in nsSVGDisplayContainerFrame::RemoveFrame.
It looks like removing this doesn't cause any failures on Try.
Comment on attachment 9012558 [details]
Bug 1494604. Remove nsSVGUtils::NotifyAncestorsOfFilterRegionChange. r?longsonr

Robert Longson [:longsonr] has approved the revision.
Attachment #9012558 - Flags: review+
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2073ec5bcc4e
Remove nsSVGUtils::NotifyAncestorsOfFilterRegionChange. r=longsonr
https://hg.mozilla.org/mozilla-central/rev/2073ec5bcc4e
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
status-firefox64: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.