creating too many popup listeners?

RESOLVED FIXED in mozilla0.9.6

Status

defect
P2
normal
RESOLVED FIXED
18 years ago
15 years ago

People

(Reporter: sspitzer, Assigned: paulkchen)

Tracking

({perf})

Trunk
mozilla0.9.6
x86
Other

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 6 obsolete attachments)

creating too many popup listeners.  (his number was 57 for the browser window).

talking to dave, he mentioned that he thinks we're creating too many pop 
listeners.

possibly creating them inside of generated content (like that other bug where 
we were putting onclick and oncommand handlers in generated content).

helping with browser performance, I'll take a look and see if this is the 
case.  (in browser and in the rest of the product.)
0.9.1, I hope.
Target Milestone: --- → mozilla0.9.1
moving to 0.9.2, 0.9.1 is too soon.
Status: NEW → ASSIGNED
Target Milestone: mozilla0.9.1 → mozilla0.9.2
I don't expect to fix this in 0.9.1, but it's one of my top priorities for 
overal performance improvements.  so focusing on it now.
Summary: creating too many popup listeners → creating too many popup listeners?
Target Milestone: mozilla0.9.2 → mozilla0.9.1

Updated

18 years ago
Blocks: 49141

Comment 4

18 years ago
moving to 0.9.2 
Target Milestone: mozilla0.9.1 → mozilla0.9.2

Comment 5

18 years ago
any ideas on what kind of improvement we expect fixing this to make?

Comment 6

18 years ago
we expect to help improve window open performance 

Comment 7

18 years ago
adding PDT+
Whiteboard: [PDT+]

Comment 8

18 years ago
moving to 0.9.3
Target Milestone: mozilla0.9.2 → mozilla0.9.3

Updated

18 years ago
Keywords: perf

Updated

18 years ago
Whiteboard: [PDT+]

Comment 9

18 years ago
moving to 0.9.4
Target Milestone: mozilla0.9.3 → mozilla0.9.4
slide to 0.9.5
Target Milestone: mozilla0.9.4 → mozilla0.9.5
back to trudelle.  this would be worth investigating, I just don't have time to
help.
Assignee: sspitzer → trudelle
Status: ASSIGNED → NEW
I'll let trudelle set the target milestone.
Target Milestone: mozilla0.9.5 → ---

Comment 13

18 years ago
->pchen for perf analysis and triage
Assignee: trudelle → pchen
(Assignee)

Comment 14

18 years ago
marking p2 and mozilla0.9.6
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.9.6
(Assignee)

Comment 15

18 years ago
After talking with Hyatt, it looks like we're overusing tooltip="aTooltip" too
far down the xul hierarchy (i.e on each toolbar button as opposed to just once
on the toolbar itself). After hacking around some, I got the number of observers
down to 43 or so just by moving tooltip="aTooltip" up the hierarchy. However,
Hyatt told me that with bug 93839, we will do away with having to specify
tooltip="aTooltip" and just create one popuplistener that knows all the
tooltiptexts. Sounds great, and it's a 0.9.6 bug to boot. So I am going to mark
this as a dup of that bug since that appears to be the right fix.

*** This bug has been marked as a duplicate of 93839 ***
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → DUPLICATE
(Assignee)

Comment 16

18 years ago
Sorry for the spam, hyatt wants me to clean this up because it will make his job
easier for 93839. I'll try to catch more of these before I post a patch
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
(Assignee)

Comment 22

18 years ago
Assuming I did this right under linux, I count *only* 34 popup listeners being
created now. Well, it's a jump in the right direction. ;-)
Status: REOPENED → ASSIGNED
Comment on attachment 53363 [details] [diff] [review]
updated navigator.xul patch, moved tooltip="aTooltip" up from first hbox in nav-bar to toolbar itself to enable tooltips in print and throbber

r=jag
Attachment #53363 - Flags: review+
Comment on attachment 53226 [details] [diff] [review]
remove tooltip="aTooltip" from offline-status because it's now provided by statusbar

r=jag
Attachment #53226 - Flags: review+
Comment on attachment 53225 [details] [diff] [review]
remove tooltip="aTooltip" since it's now provided by statusbar

r=jag
Attachment #53225 - Flags: review+
Comment on attachment 53224 [details] [diff] [review]
remove tooltip="aTooltip" since it's now provided by statusbar

r=jag
Attachment #53224 - Flags: review+

Updated

18 years ago
Attachment #53223 - Attachment is obsolete: true

Updated

18 years ago
Attachment #53180 - Attachment is obsolete: true
(Assignee)

Updated

18 years ago
Attachment #53224 - Attachment is obsolete: true
(Assignee)

Updated

18 years ago
Attachment #53225 - Attachment is obsolete: true
(Assignee)

Updated

18 years ago
Attachment #53226 - Attachment is obsolete: true
(Assignee)

Updated

18 years ago
Attachment #53363 - Attachment is obsolete: true
(Assignee)

Comment 29

18 years ago
Ok, latest fix. I don't change anything in the statusbar anymore because that
would require me to go modify every other window in the app so that their
statusbars have tooltip="aTooltip". Hyatt will just have to go fix that when he
lands his bug. ;-)

Comment 30

18 years ago
Comment on attachment 54081 [details] [diff] [review]
latest patch, navigator.xul only, don't fiddle with statusbar

sr=alecf
Attachment #54081 - Flags: superreview+

Comment 31

18 years ago
Comment on attachment 54081 [details] [diff] [review]
latest patch, navigator.xul only, don't fiddle with statusbar

r=blake
Attachment #54081 - Flags: review+
(Assignee)

Comment 32

18 years ago
fix checked into trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.