293542 - Document confusion in NS_GetSVGMarkerFrame

Status: RESOLVED WORKSFORME

(Core :: SVG, defect)

Description

[Boris Zbarsky [:bzbarsky]]

NS_GetSVGMarkerFrame seems to be using ownerDoc where it should be using
currentDoc, in some places.  In particular, we want to be calling getElementById
on the _ownerDoc_ of the aContent.  Then we should be getting the presshell from
the currentDoc of the element that returns to get the right frame.

At the moment, these are all the same document so things are ok.  Bug if we
actually implement sXBL, they won't be....

Comment 1

[Jonathan Watt [:jwatt]]

I think there's something I don't understand. The only difference between GetOwnerDoc and GetCurrentDoc is that the latter returns null if the node is not in the document tree, and GetElementById will only return an element that's in the tree. Since the element returned by GetElementById must be in the doc, why does it matter whether we call GetCurrentDoc or GetOwnerDoc on that element to get the presShell?

Comment 2

[Boris Zbarsky [:bzbarsky]]

> The only difference between GetOwnerDoc and GetCurrentDoc is that the latter
> returns null

Right now, yes.  But conceptually, GetOwnerDoc is the DOM ownerDocument (modulo the Document node silliness) while GetCurrentDoc is the document the layout objects for the node are attached to (the document being presented).  With XBL2 they're different documents for the XBL-bound anonymous content: the former is the XBL document while the latter is the document the XBL is bound to.  So for example, GetOwnerDoc() will never have a presshell for XBL anon content once XBL2 is implemented.

Note that I said this in comment 0, second paragraph.

Comment 3

[Jonathan Watt [:jwatt]]

Well, you alluded to the fact that XBL would change things. I was essentially stating my current understanding of things, inviting you to correct me where I was wrong and fill me in on what I didn't know. Thanks for expanding on the XBL point.

So NS_GetSVGMarkerFrame is long gone, but I thought I'd take a look through the GetOwnerDoc and GetCurrentDoc calls in the svg directories. I'm still not sure about some of them, but it seems like at least the GetOwnerDoc() in nsSVGAElement::IsLink() is wrong.

Comment 4

[Boris Zbarsky [:bzbarsky]]

No, that one is correct.  The charset to use is the one the node was "parsed from", which means the owner document.

Comment 5

[Robert Longson [:longsonr]]

This code was rewritten as part of the live id changes in Firefox 3.5. Should we just close this as WFM?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Can you point me to the new code?

Comment 7

[Robert Longson [:longsonr]]

NS_GetSVGMarkerFrame disappeared in bug 376611. This code was then itself replaced in bug 309220. 

bug 309220 builds on top of bug 455984 which implements the basic live Ids code and your bug to make Id look-up work with external documents.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Ah, ok.  That code looks correct, yes.  WFM sounds like a plan.