Closed Bug 111411 Opened 23 years ago Closed 6 years ago

clear Inspector when target document is destroyed

Categories

(Other Applications :: DOM Inspector, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: hewitt, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

If the document being inspected is destroyed (via the user closing its window, browsing to another page, etc...) the Inspector should release it and go back to inspecting nothing. Also, I should stop holding a reference to the document in inDOMView.
Status: NEW → ASSIGNED
Target Milestone: --- → Future
If the HTML document is destroyed or changed, but the remaining XUL DOM still exists for that window, it should just go back to the parent node of the HTML element, not clear the entire window. Still, I don't feel this should be done automatically since people might want to still look at DOM values even after the document is closed. What if you had a "refresh" button that reloaded the DOM of that page and attempted to return to the closest identical element on the DOM (for instace the parent of the HTML element, like XUL:TABBROWSER)? See also bug 181261. Since search is broken on DOM inspector, it is a pain in the neck to get back to the node you were looking at on the HTML document.
*** Bug 200890 has been marked as a duplicate of this bug. ***
Default owner. Should we have an nsIDocumentObserver notification for this sort of thing? The DocumentWillBeDestroyed is not useful, since it will not fire if you hold a ref to the doc (as JS might till the next GC, for example).
Assignee: hewitt → caillon
Severity: normal → critical
Status: ASSIGNED → NEW
Target Milestone: Future → ---
Maybe the right thing to do is to simply attach an "unload" event listener to the window the inspectee lives in?
Adding signature and keyword from dupe.
Keywords: crash
Summary: clear Inspector when target document is destroyed → clear Inspector when target document is destroyed [@ nsCSSSelector::ToString]
*** Bug 203369 has been marked as a duplicate of this bug. ***
This problem is bugging me a lot while developing and testing webapps. Is there a workaround to prevent the crashes without closing the DOM inspector?
After a quick readover of the code, and based on my work for bug 156072 and bug 193942, I can definitely state a procedure which should work. Here's what currently happens to set a document in Inspector. (1) inspector object (which will henceforth be "this") calls this.setTargetDocument(aDoc). (2) this.setTargetDocument(aDoc) sets this.mDocPanel.subject(aDoc). (3) inspector.xml#panel binding for subject calls setSubject method (4) this.mDocPanel.setSubject(aObject) (here still aDoc) searches viewer-registry.rdf for viewers which are valid for that node. (5) A default viewer is selected, from the available viewers, for the left panel. (6) Said viewer picks a selection and tells somebody about it: this.mDocPanel.viewer.mObsMan.dispatchEvent("selectionChange", { selection: this.mDocPanel.viewer.mSelection } ); (7) Somewhere along the line (I haven't figured out where), the right panel gets its subject from the left panel's selection. bz suggested an unload event listener on the doc. Such should follow this sequence: function(evt) { this.mDocPanel.viewer.mObsMan.dispatchEvent("selectionChange", { selection: null } ); // killing the right-side viewers this.setTargetDocument(null); // killing the left side viewers } We also need to change mozilla/extensions/inspector/resources/content/res/viewer-registry.rdf on the last viewer's ins:filter attribute. Currently it reads "return true;". It should read: "return object != null". That'll make sure the JSObject viewer doesn't show up for null, and save on some processor time. Only question I have is which window object's scope would the evt object fall under... as the inspected document won't have an inspector object to call all this on.
If the unload event listener does turn out to be on the inspected document's defaultView scope, could we get away with adding an "inspectorWindow" property to the inspected document?
Attached file work-in-progress (obsolete) (deleted) —
This patch works, mostly, but causes a couple unacceptable regressions. (1) Open DOM Inspector via Tools, Web Development, DOM Inspector, then tell it to inspect itself. Error: [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIRDFContainerUtils.IndexToOrdinalResource]" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: chrome://inspector/content/jsutil/rdf/RDFU.js :: anonymous :: line 62" data: no] Source File: chrome://inspector/content/jsutil/rdf/RDFU.js Line: 62 (2) Inspect a window (specifically a browser window), then navigate to a new page in that window. Inspector, which was looking at navigator.xul, now fires the unload event handler.
Mass re-assigning bugs to dom.inspector@extensions.bugs
Assignee: caillon → dom.inspector
Okay, I've done a little research, and this bug may (or may not) be fixable based on the DOM 2 Events model. Every single document which DOM Inspector looks at goes through inspector.js @ setTargetDocument as an argument. So, in theory, we should be able to detect an unload event dispatched to either the document (which would be ideal and very simple) or to a simple, common node or window related to the document. DOM Inspector can look at a document in three ways: by directly loading a URL into itself, by inspecting the document as loaded from the browser via CTRL+SHIFT+I, or by inspecting any registered application such as the browser itself. A document can be unloaded either by closing the application which contains it (this may be Inspector under the first scenario) or by loading a new URL into the container application. So there are six possibilities to consider. Here are some brief results, going from about: to about:mozilla, then closing the application. (1) DOM-I as container, navigates to new URL: evt.target = unloaded document (2) DOM-I as container, closes: evt.target = unloaded document (3) DOM-I looking at document via CTRL+SHIFT+I, navigates to new URL: evt.target = unloaded document (4) DOM-I looking at document via CTRL+SHIFT+I, tab closes: JavaScript Exception: the target of the event has no properties (probably means the document is null before the event reaches its listener, not a good thing. We also get assertions, at nsWebShell.cpp line 289 and sometimes at nsGenericHTMLElement.cpp line 4483.) (5) DOM-I looking at Mozilla Navigator, navigates to new URL: evt.target = <xul:tabbrowser/>. (6) DOM-I looking at Mozilla Navigator, application closes: multiple unload events, 1 for each tabbrowser, and one for the Navigator XUL document. In the first three cases, the document being unloaded is the target, and this is very conveinient for DOM Inspector. In the fourth case, we have an exception and a bunch of assertions. In the last two cases, the event stops just shy of the document being unloaded... which is inconsistent with the first three cases. Note bug 99820 for some related information. setTargetDocument: function(aDoc) { try { if (this.mDocPanel.subject) { this.mDocPanel.subject.defaultView.removeEventListener("unload", DocumentUnloadListener, true); dump("Event listener removed for " + this.mDocPanel.subject.location + "\n"); } } catch(e) { dump("setTargetDocument removeEventListener error: " + e.msg + "\n"); } this.mDocPanel.subject = aDoc; try { if (aDoc) { aDoc.defaultView.addEventListener("unload", DocumentUnloadListener, true); dump("Event listener added for " + aDoc.location + "\n"); } } catch(e) { dump("setTargetDocument addEventListener error: " + e.msg + "\n"); } }, // then, at the end of the script: function DocumentUnloadListener(aEvent) { dump(aEvent.target); if (aEvent.target.nodeType == 1) { dump(" (" + aEvent.target.nodeName + ")"); } else if (aEvent.target.nodeType == 9) { dump(" (" + aEvent.target.location + ")"); } dump(" unload event at " + aEvent.currentTarget + "\n"); }
> (5) DOM-I looking at Mozilla Navigator, navigates to new URL: evt.target = > <xul:tabbrowser/>. Because of XBL. Try using evt.originalTarget instead.
Heh, oops. I should've thought of that. Okay, that solves five of six scenarios. The only one left, number 4, still throws assertions, and evt.originalTarget === null. caillon: module owner's question. I now have enough information to fix this bug, and by doing so, we'll almost certainly close two other critical bugs in DOM Inspector. My question is, should I wait for the bug I'm seeing in scenario 4 to be fixed? I can rig the code to accept evt.originalTarget === null and dump() something to warn people this is happening.
Alex: Is there any way that when the document is reloaded and you are within that document on DOM inspector (such as within an HTML document) that we can return to the node before the part of the DOM that was reloaded? Sorry if this question is redundant to something you already explained, I just want to make sure its not reset to the very beginning of the DOM as I mentioned in comment #1. For instance, if the HTML page is reloaded, we could change the selected tree member in DOM Inspector to the tab panel, not the very beginning of the DOM. That would be easier for people that are inspecting a document's DOM and constantly reloading the document within the tab, etc.
I don't see how that's possible, or even safe. When the document is reloaded, that means the current document is destroyed and a new document (pulled from the same URL) is created. With the destruction of the current document comes the destruction of every single node beneath it. Any attempts to reference those nodes is a clear invitation to crash (bug 128110, bug 219514). Your comment 1 in this bug is nice, but not immediately doable John Keiser's In bug 181261 comment 2, jkeiser offers a possible solution, but it's not nearly as important as stopping the crashes. Believe me, I sympathize a great deal; I know exactly what you are talking about and I'm constantly reloading too. But this is not the place to raise that issue.
Note bug 224741 for what I saw in comment 12, scenario 4. Hopefully, we'll get some traction there.
I don't quite consider this ready for reviews yet, for two reasons. (1) I'm getting a lot of XPConnect errors. Although they don't appear to affect the operation of DOM Inspector, I would like to figure out how to shut them down. (2) This patch depends partly on a maintenance patch for bug 193942. The reason is the left-hand panel with this patch gets its subject set to null. This means the JavaScript Object viewer takes over on the left side, and the right-side panel's subject never gets updated to reflect this. I'd really appreciate it if a few volunteers would do some serious testing of this patch, and offer suggestions for nailing down the XPConnect exceptions.
Attachment #125679 - Attachment is obsolete: true
>(2) This patch depends partly on a maintenance patch for bug 193942. The >reason is the left-hand panel with this patch gets its subject set to null. >This means the JavaScript Object viewer takes over on the left side, and the >right-side panel's subject never gets updated to reflect this. I changed line 89 of viewer-registry.rdf to "return object instanceof Object;"
Heh, that doesn't unload the left panel at all :-)
*** Bug 244285 has been marked as a duplicate of this bug. ***
*** Bug 266732 has been marked as a duplicate of this bug. ***
Product: Core → Other Applications
Assignee: dom-inspector → nobody
QA Contact: timeless → dom-inspector
Crash Signature: [@ nsCSSSelector::ToString]
Alex (or anyone else here), does this still happen? Do we need to track this, maybe get the patch in, or should we close the bug?
I don't remember the application this bug applies to, so I made a little test in both browsers. Firefox: I don't know if the new(?) inspector is the same this bug relates to, but it is closed when a link is clicked, so the problem is worked around. Seamonkey: It uses the "classic" DOM inspector. The DOM being inspected is not destroyed / updated / resetted but I can't detect any crash when working with nodes (cut, paste and delete works ok; blink element does nothing -naturally-). The DOM is updated when it is changed using javascript, no crashes. I think this bug should be closed, but it would be nice if DOM inspector notifies the user [*] when the rendered document is not visible anymore (tab or window closed or navigated out) [*] May it use the yellow top bar like popup blocker do?
> I don't remember the application this bug applies to See the "product" and "component" fields on the bug?
"Product: Other Applications" did not help.
Well, the Firefox inspector is part of the "Firefox" product; it's not a separate application....
I am sorry I think it was a separate application in 2001 when this bug was reported, but I am not sure if I can remember correctly. Just ignore my references to Firefox, please.
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #23) > Alex (or anyone else here), does this still happen? Do we need to track > this, maybe get the patch in, or should we close the bug? This is a crash bug, but the suggestions about what to do when documents are unloaded are pertinent enough for an enhancement bug. FWIW, I inspect unloaded documents all the time (with XBL even), and it hasn't been prone to cause crashes, at least not over the last few years using release builds. If you're comfortable with that, I'd say remove the crash bits from this bug or close it, and the enhancement suggestions can live in a new bug.
(In reply to Colby Russell :crussell from comment #29) > FWIW, I inspect unloaded documents all the time (with XBL even), and it > hasn't been prone to cause crashes, at least not over the last few years > using release builds. If you're comfortable with that, I'd say remove the > crash bits from this bug or close it, and the enhancement suggestions can > live in a new bug. I'll leave it up to you if you want out handle the enhancements in this or in a new bug, I'm removing the crash stuff from here at least, everything else is up to you as the DOMi maintainer. :)
Severity: critical → enhancement
Crash Signature: [@ nsCSSSelector::ToString]
Keywords: crash
Summary: clear Inspector when target document is destroyed [@ nsCSSSelector::ToString] → clear Inspector when target document is destroyed
Bulk close. This component is no longer supported or maintained. https://bugzilla.mozilla.org/show_bug.cgi?id=1499023
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INCOMPLETE
Bulk close. This component is no longer supported or maintained. https://bugzilla.mozilla.org/show_bug.cgi?id=1499023
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: