355114 - xml:space handling mixes GetCurrentDoc and GetOwnerDoc

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Robert Longson [:longsonr]]

See bug 319786 comment 29 onwards.

Comment 1

[Robert Longson [:longsonr]]

Attachment: patch

Comment 2

[Boris Zbarsky [:bzbarsky]]

What's the lifetime of the SVG mutation observer?  Does it basically die when the frame dies?

Comment 3

[Robert Longson [:longsonr]]

(In reply to comment #2)
> What's the lifetime of the SVG mutation observer?  Does it basically die when
> the frame dies?
> 

It's static, there's only one. It needs to be attached when the outer SVG frame is created and detached when it the outer frame is destroyed.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Then how does this patch work, exactly?

Comment 5

[Robert Longson [:longsonr]]

(In reply to comment #4)
> Then how does this patch work, exactly?
> 

I was trying to guard against the mutation observer being added multiple times for in the same document but there are two issues with that

a) NodeWillBeDestroyed is only called when the document is destroyed and not on frame destruction.

b) nsBindingManager::AddObserver already checks that the observer is not already added before adding it.

I now think that the best thing to do is just to remove the code in the nsSVGOuterFrame destructor - there is only one nsSVGMutationObserver which can only be added at most once to any one document. Removing it is not necessary.

Comment 6

[Robert Longson [:longsonr]]

Attachment: updated patch

Comment 7

[tor]

Comment on attachment 240939 [details] [diff] [review]
updated patch

Maybe add a comment at the point you add the observer explaining why it's never explicitly removed?

Comment 8

[Robert Longson [:longsonr]]

Attachment: address review comment

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 242614 [details] [diff] [review]
address review comment

Hmmm.... ok.

Comment 10

[Robert Longson [:longsonr]]

checked in