DOM Inspector leaks (dom.js)

RESOLVED FIXED

Status

Other Applications
DOM Inspector
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: JasnaPaka, Assigned: Régis Caspar)

Tracking

({fixed-seamonkey1.1a, fixed1.8.1, mlk})

1.8 Branch
fixed-seamonkey1.1a, fixed1.8.1, mlk
Bug Flags:
blocking1.8.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

9.56 KB, image/png
Details
Fix
1021 bytes, patch
jag (Peter Annema)
: review+
neil@parkwaycc.co.uk
: superreview+
Mike Schroepfer
: approval1.8.1+
Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
: approval-seamonkey1.1a+
Details | Diff | Splinter Review
1.21 KB, text/plain
Details
(Reporter)

Description

12 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060506 Minefield/3.0a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060506 Minefield/3.0a1

Firefox Trunk, clean profile.

Reproducible: Always

Steps to Reproduce:
1. Install Leak Monitor Extension (http://dbaron.org/mozilla/leak-monitor/) and
Nightly Tester Tools (https://addons.mozilla.org/firefox/958/).
2. Close Firefox.
3. Open Firefox, open DOM Inspector (Tools -> DOM Inspector), close DOM Inspector.

Actual Results:  
Leak alert is showed.
(Reporter)

Updated

12 years ago
Keywords: mlk
(Reporter)

Comment 1

12 years ago
Created attachment 221144 [details]
Leak Monitor Extension - Result

Comment 2

12 years ago
this isn't useful. 1. devs shouldn't be forced to load huge images that ff auto shrinks. if you can't retype the text into the bug, and if your extension can't copy the text for you (which you should file a bug about), then please don't file a bug about it.

oh, and i hope you realize that no one is actively working on this component, so if you want something done about it, you're expect to write the patch yourself (or not use the extension).

Comment 3

12 years ago
I can confirm this with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20060507 BonEcho/2.0a1.

OS -> All?
We add a pref observer at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/extensions/inspector/resources/content/viewers/dom/dom.js&rev=1.38&mark=80#79 and never remove it.
Keywords: helpwanted
(Assignee)

Comment 5

12 years ago
Created attachment 221243 [details] [diff] [review]
Fix

Add a unload routine to remove the pref observer
Attachment #221243 - Flags: review?
(Assignee)

Updated

12 years ago
Attachment #221243 - Flags: review? → review?(jag)

Updated

12 years ago
Attachment #221243 - Flags: superreview?(neil)
Attachment #221243 - Flags: review?(jag)
Attachment #221243 - Flags: review+

Comment 6

12 years ago
Comment on attachment 221243 [details] [diff] [review]
Fix

> function DOMViewer_initialize()
> {
>@@ -61,6 +62,12 @@
>   viewer.initialize(parent.FrameExchange.receiveData(window));
> }
Note to patch author: sometimes it helps to specify a non-default -u value.
Attachment #221243 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 7

12 years ago
(In reply to comment #6)
> Note to patch author: sometimes it helps to specify a non-default -u value.
Yes, I will pay attention to this the next time. Thanks

Updated

12 years ago
Assignee: dom-inspector → regis.caspar+bz
OS: Windows XP → All
Hardware: PC → All
Whiteboard: [checkin needed]
Version: unspecified → 1.8 Branch

Updated

12 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 8

12 years ago
 -> updated keyword / ASSIGNED.

Is it something missing or this is just waiting for someone to check this in?
Status: NEW → ASSIGNED
Keywords: helpwanted
mozilla/extensions/inspector/resources/content/viewers/dom/dom.js 	1.40
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: --- → mozilla1.9alpha
Version: 1.8 Branch → Trunk
(Assignee)

Comment 10

12 years ago
VERIFIED on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060530 Minefield/3.0a1 ID:2006053004 [cairo]

Will ask for branch approval in a few days.
(Assignee)

Comment 11

12 years ago
(In reply to comment #10)
> Will ask for branch approval in a few days.

This seems to need to be checked-in in branch but I don't know where/who to ask for approval/check-in. Any direction would be great :)

Attachment #221243 - Flags: approval-seamonkey1.1a?
Attachment #221243 - Flags: approval-seamonkey1.1a? → approval-seamonkey1.1a+
Attachment #221243 - Flags: approval1.8.1?

Comment 12

12 years ago
*** Bug 333364 has been marked as a duplicate of this bug. ***
Transferring blocking flag from duplicate.
Flags: blocking1.8.1+

Updated

12 years ago
Attachment #221243 - Flags: approval1.8.1? → approval1.8.1+
I found another leak in DOMi

Reproducible: Always

Steps to Reproduce:
1. Install Leak Monitor Extension (http://dbaron.org/mozilla/leak-monitor/)
2. Restart Firefox.
3. Open DOM Inspector (Tools -> DOM Inspector)
4. From DOM Inspector window, click the Select Element By Click button.
5. Close DOM Inspector without selecting any object/element.

Actual Results:
Leak alert is showed (see attachment).

Tested on:
- User Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4 Build ID: 2006050817
- User Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060526 BonEcho/2.0a3 Build ID: 2006052616
Created attachment 226862 [details]
DOM Inspector leak

Solution:
Add these before viewer = null; to function DOMViewer_destroy() in the patch:

try {
  viewer.removeClickListeners();
} catch(ex) {
}

Comment 16

12 years ago
LouCypher, please test again after this patch actually lands on the 1.8 branch. Then if you can still reproduce, file a new bug.
(In reply to comment #16)
> LouCypher, please test again after this patch actually lands on the 1.8 branch.
> Then if you can still reproduce, file a new bug.
> 

Looks like I have to file a new bug.
Not in this weekend, though.

Whiteboard: [checkin needed]
mozilla/extensions/inspector/resources/content/viewers/dom/dom.js 	1.35.4.2
Keywords: fixed-seamonkey1.1a, fixed1.8.1
Whiteboard: [checkin needed]
Target Milestone: mozilla1.9alpha → ---
Version: Trunk → 1.8 Branch
Bug 344545 reported.
QA Contact: timeless → dom-inspector
You need to log in before you can comment on or make changes to this bug.