Closed Bug 93902 Opened 19 years ago Closed 11 years ago

about:bloat, about:bloat?new are broken

Categories

(Core :: XPCOM, defect)

Product:
Core
Component:
XPCOM
Type:
defect
Priority:
Not set

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9.1b2

People

(Reporter: megabyte, Assigned: sgautherie)

References

(
URL
)

Details

Attachments

(3 files, 2 obsolete files)

(Av3) Hide the sub-menu [Checkin: Comment 15]
4.36 KB, patch
neil
: review+
neil
: superreview+
Details | Diff | Splinter Review
(Bv1) Disable the sub-menu [Checkin: Comment 19]
3.17 KB, patch
neil
: review+
neil
: superreview+
Details | Diff | Splinter Review
(Cv1) Ignore windows which don't get the Debug menu [Checkin: Comment 28]
2.17 KB, patch
neil
: review+
Details | Diff | Splinter Review 
about:bloat and variants only give (in plain-text) <html><body></body></html>
Win2K/2001080508-trunk
I also see this 2001080508 Win98SE
This isn't networking, but I have no idea who owns this.

-> xpcom, I guess

-> waterson, maybe?
Assignee: neeti → waterson
Component: Networking → XPCOM
qa -> component
Assignee: waterson → kandrot
QA Contact: benc → scc
I think this is a dup of bug 87513.
Do you agree?
No, about:bloat is actually supposed to do something.
about:bloat?clear actually seems to work
updating summary
Summary: about:bloat is broken → about:bloat, about:bloat?new are broken
I believe waterson wrote this, so I am reassigning to him.  If not him, at 
least he will know about it.
Assignee: kandrot → waterson
Target Milestone: --- → Future
Status: NEW → ASSIGNED
Assignee: waterson → nobody
Status: ASSIGNED → NEW
QA Contact: scc → xpcom
Works fine for me in a Mac trunk debug build, with trace-refcnt compiled in and enabled.
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081016 SeaMonkey/2.0a2pre] (nightly) (W2Ksp4)

See bug 451291 comment 4.

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081019 SeaMonkey/2.0a2pre] (home, optim default) (W2Ksp4)
(with |ac_add_options --enable-logrefcnt|)

The two options don't seem to do anything !?
Blocks: 451291
Target Milestone: Future → ---
Attached patch (Av1-WIP) Hide the menu items (obsolete) — — Splinter Review 
(In reply to comment #9)
> See bug 451291 comment 4.

This patch works fine, except
the vbox changes the text highlight color to black (instead of white) :-/

Ideas:
*Add an "id" on each items, instead of a vbox :-(
*Move the Bloat items to a sub-menu, as the LeakDetector items are :-|
*Add a fixing style to the vbox :-)

What do you think ?
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #343813 - Flags: review?(neil)
Comment on attachment 343813 [details] [diff] [review]
(Av1-WIP) Hide the menu items

Sorry, you can't put a menuitem in a box, but <observes element="bloatSeparator" attribute="hidden"> avoids hiding them individually.
Attachment #343813 - Flags: review?(neil) → review-
Attached patch (Av2) Hide the menu items (obsolete) — — Splinter Review 
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081016 SeaMonkey/2.0a2pre] (nightly) (W2Ksp4)

(In reply to comment #11)
> (From update of attachment 343813 [details] [diff] [review])
> <observes element="bloatSeparator" attribute="hidden"> avoids hiding them individually.

Nice feature; but I would prefer not to add "observers" for a one-time (per window) call not happening in nightly/release builds :-|
Attachment #343813 - Attachment is obsolete: true
Attachment #343827 - Flags: superreview?(neil)
Attachment #343827 - Flags: review?(neil)
(In reply to comment #10)
> *Add an "id" on each items, instead of a vbox :-(
> *Move the Bloat items to a sub-menu, as the LeakDetector items are :-|
I see your current patch implements the :-( method - would you mind writing a patch for the :-| method just to see how much better it might be?
Per comment 13 ;-)

This is my preferred solution, but I don't know if adding a sub-menu is an annoyance or not to people would use this feature.
Attachment #343857 - Flags: superreview?(neil)
Attachment #343857 - Flags: review?(neil)
Attachment #343857 - Flags: superreview?(neil)
Attachment #343857 - Flags: superreview+
Attachment #343857 - Flags: review?(neil)
Attachment #343857 - Flags: review+
Attachment #343827 - Attachment is obsolete: true
Attachment #343827 - Flags: superreview?(neil)
Attachment #343827 - Flags: review?(neil)
Flags: in-testsuite-
Keywords: checkin-needed
OS: Windows 2000 → All
Hardware: PC → All
Whiteboard: [c-n: leave open]
Target Milestone: --- → mozilla1.9.1b2
Comment on attachment 343857 [details] [diff] [review]
(Av3) Hide the sub-menu
[Checkin: Comment 15]

http://hg.mozilla.org/comm-central/rev/cf14a12c282e
Attachment #343857 - Attachment description: (Av3) Hide the sub-menu → (Av3) Hide the sub-menu [Checkin: Comment 15]
(In reply to comment #9)
> [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081016
> SeaMonkey/2.0a2pre] (nightly) (W2Ksp4)
> 
> See bug 451291 comment 4.

The URLs were already detected as invalid;
the menu is now hidden too.

> [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081019
> SeaMonkey/2.0a2pre] (home, optim default) (W2Ksp4)
> (with |ac_add_options --enable-logrefcnt|)
> 
> The two options don't seem to do anything !?

That's expected: to actually display the BloatView, see
https://developer.mozilla.org/en/Debugging_memory_leaks#How_to_turn_on_refcnt.2Fmemory_logging
Keywords: checkin-needed
Whiteboard: [c-n: leave open]
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081022 SeaMonkey/2.0a2pre] (home, debug default) (W2Ksp4)

Fine-tune the UI:
when the feature is not currently active, still unhide the menu, but disable it.
Attachment #345017 - Flags: superreview?(neil)
Attachment #345017 - Flags: review?(neil)
Attachment #345017 - Flags: superreview?(neil)
Attachment #345017 - Flags: superreview+
Attachment #345017 - Flags: review?(neil)
Attachment #345017 - Flags: review+
Comment on attachment 345017 [details] [diff] [review]
(Bv1) Disable the sub-menu
[Checkin: Comment 19]

Excellent :-)
Comment on attachment 345017 [details] [diff] [review]
(Bv1) Disable the sub-menu
[Checkin: Comment 19]

http://hg.mozilla.org/comm-central/rev/a734a91a43f4
Attachment #345017 - Attachment description: (Bv1) Disable the sub-menu → (Bv1) Disable the sub-menu [Checkin: Comment 19]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Oops, looks like this is busted in Navigator's version of view source.
(In reply to comment #20)
> Oops, looks like this is busted in Navigator's version of view source.

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081112 SeaMonkey/2.0a2pre] (nightly) (W2Ksp4)

Not seeing anything wrong (with a nightly...).
Steps ?
Well, you wouldn't see a problem with a nightly, but if you had a build that was capable of supporting about:bloat or the leak detector then you'd get an error message opening the view source window.
Error: document.getElementById("bloatAndLeakSeparator") is null
Source File: chrome://debugqa/content/debugQAMenuOverlay.js
Line: 68
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081113 SeaMonkey/2.0a2pre] (home, optim default) (W2Ksp4)
Attachment #348170 - Flags: review?(neil)
Comment on attachment 348170 [details] [diff] [review]
(Cv1) Ignore windows which don't get the Debug menu
[Checkin: Comment 28]

I think it would be better if those onload handlers don't get to run in the first places.
Attachment #348170 - Flags: review?(neil) → review-
(In reply to comment #25)
> first places.
Bah, I blame bug 382187 ;-)
Comment on attachment 348170 [details] [diff] [review]
(Cv1) Ignore windows which don't get the Debug menu
[Checkin: Comment 28]

Sorry, I was thinking of something else.
Attachment #348170 - Flags: review- → review+
Comment on attachment 348170 [details] [diff] [review]
(Cv1) Ignore windows which don't get the Debug menu
[Checkin: Comment 28]

http://hg.mozilla.org/comm-central/rev/fc2d9e140d70
Attachment #348170 - Attachment description: (Cv1) Ignore windows which don't get the Debug menu → (Cv1) Ignore windows which don't get the Debug menu [Checkin: Comment 28]
You need to log in before you can comment on or make changes to this bug.