Closed Bug 391177 Opened 15 years ago Closed 12 years ago
XSLT should reuse the inner window
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7) Gecko/2007080210 GranParadiso/3.0a7 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7) Gecko/2007080210 GranParadiso/3.0a7 In trying to support web pages generated by XSLT in Firebug I discovered that the window object associated with the .xml URL changed during the XSLT execution in ways other than transforming the document. Specifically I found that my eventListeners never fired and my window properties disappeared. Jonas Sicking says: "After some digging and help from jst we determined that what happens is that we once the xslt execution starts remove all registered eventhandlers and remove all set properties on the window. " The direct impact is that Firebug can't know when the window is ready to be debugged. In general all kinds of Firefox components and extensions watch windows by adding event handlers. Reproducible: Always Steps to Reproduce: 1. To build a complete focused test case I think one needs to build a Firefox extension that sets an eventListener on the xml window and a property then inspects them at the end. As an alternative, Firebug branches/explore has a trace option that will expose the issue. Let me know if this is useful and I will send a XPI file and a trace from the current 3.0a7. Also Firebug should work on XSLT pages if this problem is fixed. 2. 3. Actual Results: Firebug can't see the XSLT result because the window eventhandlers do not fire. Expected Results: Firebug just works because the result from XSLT is a web page. There is only a "General" category, the select box does not seem to have core components. Obviously this should be under XSLT. See also mozilla.dev.tech.xslt Re: XSLT processing model in Mozilla news://news.mozilla.org:119/OKKdnQTMt_r5VirbnZ2dnUVZ_smnnZ2d@mozilla.org
15 years ago
Component: General → XSLT
Product: Firefox → Core
QA Contact: general → xslt
So is the issue just that we should be reusing the inner window when we make this SetDOMDocument call? Is that in fact what we want to be doing? If so, could we make use of the aClearScopeHint argument to SetNewWindow (which is currently unused) to indicate whether to force reusing the inner window? e.g. force reusing if the arg is false.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: breaks firebug
If we do that we should rename the argument and make its name reflect what we're really doing here, or if we don't do that please file a bug (or remind me to file the bug :) to remove that argument...
Yeah, i think that's what we want to do.
Don't think this is blocking. Adding wanted as this affects firebug though.
Actually, one issue here is that scripts in the source document will run against its inner window. Do we really want those scripts polluting the scope of the transformed document? I would argue no. Sicking, peterv, what do you think? If we don't want said pollution, I think we should really give a better way for firebug and other extensions to hook into the point when we swap documents or something, and keep clearing the scope when we do the swap. Another option is to disable the scriptloader once we know we're going to be doing an XSLT transform. Not sure whether that's the behavior we want or not.
We should definitely disable the scriptloader. I thought we already did, but i can't find any such code right now.
(In reply to comment #5) > Actually, one issue here is that scripts in the source document will run > against its inner window. Do we really want those scripts polluting the scope > of the transformed document? I would argue no. Sicking, peterv, what do you > think? I'm not sure what is the "source document" and "inner window" but debuggers want access to anything that affects the result.
Whiteboard: breaks firebug → [firebug-p2]
Version: unspecified → Trunk
My recollection from our meeting at the summit: Boris thought that Bug 342715 would provide Firebug with access to the input and output windows and we can sort out what to show the developer. We'll depend up on that one here.
Depends on: 342715
Summary: XSLT removes all registered eventhandlers and window properties. → XSLT prevents access to source window eventhandlers and window properties.
Well... the original bug summary here is still correct. We shouldn't really be wiping out the old window, since that can script up content, not just firebug. It's pretty simple to write a testcase using window.open where things don't work right in the current setup for content code. Fixing that doesn't depend on bug 342715.
No longer depends on: 342715
Summary: XSLT prevents access to source window eventhandlers and window properties. → XSLT should reuse the inner window
from MDC: "a new XML document is created based on the content of an existing document" kind of implies that a new inner window would get created for this, though I can see how that would cause scripting issues. Is this still a bug?
You can investigate if any of the issues are fixed: http://code.google.com/p/fbug/issues/list?can=2&q=Xslt but I doubt there has been any change. However there is little push to fix this.
Rob, there are certainly cases where we create a new document without creating a new inner window. See nsGlobalWindow::WouldReuseInnerWindow. And one of the XLST module peers seems to think we should be doing so here (comment 3).
Ok, this sounds like a fix is pretty do-able. Not sure how high a priority is though. Those bugs in the Firebug issue tracker are pretty old and we haven't heard anything on them for over a year. Moving this to P3. Jonas, think this is feasible for 1.9.3?
blocking2.0: --- → ?
Whiteboard: [firebug-p2] → [firebug-p3]
I think so yeah. Just gotta remind me so that it doesn't fall between the cracks. If you want me to do it that is.
well, we can evaluate after the 1.9.2 madness has ended. I guess the questions is, "is this a high enough priority?" and I'm not qualified to assess that. We can revisit later.
12 years ago
Attachment #473862 - Flags: review?(mrbkap) → review?(jst)
Comment on attachment 473862 [details] [diff] [review] patch to fix r=jst if you add an assert (or even a runtime check) that the principal of the new document in nsGlobalWindow::SetNewDocument() is the same as the old document in the case where aForceReuseInnerWindow is true.
Attachment #473862 - Flags: review?(jst) → review+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.