Link Toolbar should be converted to XBL

RESOLVED WONTFIX

Status

--
major
RESOLVED WONTFIX
18 years ago
5 years ago

People

(Reporter: hyatt, Assigned: jag+mozilla)

Tracking

({helpwanted, perf, topperf})

Trunk
helpwanted, perf, topperf
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

18 years ago
The link toolbar as written right now is impacting both startup time and new
window time because of the way it was architected.  There is XUL that is always
parsed, CSS that is always parsed, and about 900 lines of JS code that is always
loaded and compiled at startup.

The link toolbar should be an empty shell of a toolbar until the first page with
links is encountered.  At that time an attribute should be set on the toolbar
that causes it to be bound to an XBL binding for the link toolbar that will then
load in the JS and use a scoped stylesheet and anonymous content to represent
the buttons on the toolbar.

This will avoid virtually all of the overhead that the link toolbar currently
incurs and return us to the situation we were in before this new toolbar landed.
(Reporter)

Updated

18 years ago
Keywords: perf

Comment 1

18 years ago
Won't fastload more-or-less have the same effect?

Btw, I've been saying for a long time that the link toolbar should be triggered
from XBL on <link> tags rather than by JS code triggered in onload, but that's
blocked by bug 59701 because XBL bound to an html <link> tag would be untrusted.
I don't know if that's related to this bug, however - it might fix some of the
issues raised here, but probably not all of them.

Comment 2

18 years ago
PS: could you give some information about exactly what degree of performance
impact this is having?

I can't imagine it's *that* severe - I've been running with the toolbar
including autohide for months and never noticed a particular slowdown in app
launch time or new windows opening, although I admit I don't open new windows often.
FastLoad is a win, but it doesn't make the fastloaded data free to load.  So
hyatt is right to argue that all costs (reduced by FastLoad or not) should
accrue not at startup, but when you first load a page that needs the linkbar.

/be
(Reporter)

Comment 4

18 years ago
The impact is probably around 2-3% if I had to hazard a guess.  That's why I'm
not proposing anything drastic like a backout.  This is just something to do in
the future to make the feature a little cleaner.
(Reporter)

Comment 5

18 years ago
BTW, I like the idea of the <link> triggering the showing of the toolbar, and I
can tell you exactly how to do it using XBL without having to solve the security
issues.

Bind links that will cause the toolbar to show to some XBL that fires a custom
DOM event that then bubbles up into the chrome.  The toolbar (rather than
listening for load events) could listen for this custom event instead and then
manage the addition of the link/updating itself etc. when it gets this event).
(Reporter)

Comment 6

18 years ago
This is how tabs in the tabbrowser update their titles.  I made documents fire a
custom DOMTitleChanged event that the tabbrowser listens for to update its tab
titles.  You can see the relevant code in tabbrowser.xml (for the listener part)
and nsDocument::SetTitle (for the event firing part).
Hyatt, I don't think the feature should be backed out, but Gerv, et al.: I do
think this bug should be fixed ASAP.  It wasn't the last cookie I ate that made
me fat.  Startup is not slow due to any one cause.  We need to stop adding costs
to the startup critical path, and start removing costs.

/be

Comment 8

18 years ago
Hyatt: That's ingenious :) Wouldn't your custom event also be available to
remote code, though, so a malicious page could fire one manually? (Okay, I guess
that's not actually a security hole since the event can't do anything the page
couldn't do by modifying its own DOM anyway, but it still seems wrong...) Also,
can events like this still propagate before the document is fully loaded or
after its onunload has fired?

If I get a chance (no promises! real life is busy atm) I'll take a look into the
part of this bug that involves using XBL bound to <link> to populate the toolbar
instead of an onload handler. I think that would take some of the costs out of
the critical path (mostly on page-load rather than new-window), but nowhere near
as much as Hyatt's original proposal. Should this be a separate bug?

As a preliminary question that affects how I would implement it: is it true that
given an XBL binding on a tag in an HTML document, events are guaranteed to
happen in the following order: xbl-constructor, document.onload,
document.onunload, xbl-destructor?
(Reporter)

Comment 9

18 years ago
Stuart, no worries on the malicious page front, since the event's target could
be checked to ensure that it was a valid link element.  You'll have to do that
to populate the toolbar anyway.
(Reporter)

Comment 10

18 years ago
Stuart, sure, the DOMTitleChanged DOM event that i use to update tab titles
fires way before the page has loaded.  

In fact, this will make the Link toolbar kick ass, because it will be capable of
incrementally populating the toolbar, even well before the page is loaded!
(Reporter)

Comment 11

18 years ago
Let's file a separate bug for the <link> element using XBL to fire a custom DOM
event.  This bug should just cover the conversion of the toolbar itself to XBL
in order to lazy-load the methods/properties and the CSS and XUL.
QA Contact: sairuh → claudius
Blocks: 103053

Comment 12

18 years ago
This is behind the ~5% slowdown in page loadtimes on win98 and linux (Mac is 
kind of lost in the noise). I've verified the initial test results, and get 
the same sort of slowdown. I've also tested / backed-out+tested / 
reapplied+tested /backed-out-again+tested, and it is clearly this change that 
is the cause.

Okay, I'll play bad cop: this should be backed out until the new solution is 
worked out (and tested). 

"If you keep eating cookies, you'll never see your toes".
Severity: normal → major
Keywords: topperf
OS: Windows 2000 → All
Hardware: PC → All

Updated

18 years ago
Blocks: 71668
(Reporter)

Comment 13

18 years ago
I concur with jrgm.  It's one thing to slow down new window and startup by 2-3%,
but hitting page load times for 5% is unacceptable.  This could be alleviated by
eliminating the load listener and using custom DOM events and forcing the
relevant <link> elements themselves to fire the events up to the listening toolbar.
I'm the bad mozilla.org cop (donuts don't work on me).  If we've failed to stop
regressions in the past, it's because their signal in jrgm's test looked like
noise, until it piled up over too long an interval for anyone to play bonsai
hook detective and pin cvsblame on any one checkin, or day's worth of checkins.
 Here we have a smoking gun.  We have to stop regressing, now -- or we won't
make any kind of performant 1.0, soon or ever.

I have complete confidence in the interested parties being able to hack along
the lines hyatt has outlined, with hyatt's help, to restore a linkbar that does
not cost startup and page-load performance until you hit a page with link tags.
 Let's back out, get it right, and land it for 0.9.6.  Gerv, what do you say?

/be
(Reporter)

Comment 15

18 years ago
See bug 103082.  I performed a minimal excision, and only turned off the
inclusion of the overlay for now.
(Reporter)

Comment 16

18 years ago
Actually I recommend not even using XBL for the firing of the custom DOM event.
 Make it fast, don't waste time with XBL, and just fire the event from the C++
element (since we already have a class for it). 

Comment 17

18 years ago
In that case it's certainly beyond my hacking abilities and I'll have to defer
to someone more knowledgeable :(

Comment 18

18 years ago
I think bug 103097 would be a good place to do the page-load work since the
page-load-time regression was what eventually backed us out. We'll keep this bug
for just the new-window and startup improvement, per Hyatt's original
submission.

I've added gerv and hyatt to that bug, anyone else interested should hop on
over.
Somehow I get a feeling that the Link Toolbar is being held to a higher standard
than other stuff. (For example, many folks were reluctant to back out "What's
Related?" despite the privacy hole.) I agree that performance is important, but
it seems to me that keeping this in the tree makes it easier to contribute
improvements.

Comment 20

18 years ago
Why do we need custom DOM events?  Why can't we just attach a DOMNodeInserted
listener to the <head> element?  We need this anyway in case links are added
programmatically from JS.
What if a <link> is inserted into the <body> via JS?

A custom DOM event fired from the C++ implementation of the <link> node would be
fired on programmatic creation too, of course.
bz: then people will learn not to create certain invalid documents by DOM manipulation. ;)I'm adding 103060 (jwbaker's bug on DOMNodeInserted) as a dependency; also, won't we need to be listening for changes in the attributes of link elements?

Comment 23

18 years ago
Listening for changes in the link elements would be a lot easier if
DOMSubtreeModified mutation events were actually implemented.  Bug 74218.
(Reporter)

Comment 24

18 years ago
Do not beat the mutation event drum.  Mutation events will slow down page load
even more than anything else that the link toolbar is already doing.  You want a
custom DOM event that is specific to <link> elements and that fires only for
<link> elements, so you don't waste time filtering out all the noise (or taking
the global mutation event hit that occurs the moment you use a single mutation
event listener).
(Reporter)

Comment 25

18 years ago
Also, Henri, the link toolbar is absolutely not being held to a higher standard.
 Any feature that causes a 5% spike in page load times should be backed out. 
We've been strict about backing out obvious page load spikes for months.

Comment 26

18 years ago
Fine I won't beat it but we need to get the events from both <link> and <meta>.

Comment 27

18 years ago
Two thoughts about the links toolbar:
1. It should not be included in released builds, but should be made available
   as a package (jar file) that people can drop into their build if they want
   the functionality.

2. It should be reimplemented as a sidebar panel, thus avoiding the constant
   resizing of the content area as it is shown and hidden.

Comment 28

18 years ago
Those would have been good suggestions three years ago when the design
discussion took place, but I think it's a little late for dramatic changes.  In
any case, equivalent functionality in the sidebar would require significantly
more of the window area than the current toolbar requires.

As for the separate JAR, that doesn't seem like a terribly good idea unless you
also want to implement other standard HTML features in separate JARs.  Perhaps
we could just ship with a capability-free browser and then you can add the JARs
you want to just support the <p> and <ul> elements only.

Comment 29

18 years ago
I'm not in full agreement with either point.

1) Could be said for a lot of things, but this seems
like a pretty fundamental navigation tool that people
might not realize they 'want' until they've had it a
while!  It's a matter of finally making good use of
navigation data that some people have been building into
their pages practically since year dot.

2) You avoid the resize by having the toolbar always-there
or always-not-there.  There's already a menu option for
that.  You might /almost/ as validly argue that the
regular forward, backward, stop etc. buttons should go
in the sidebar (well, that's an /interesting/ thought).

Comment 30

18 years ago
Not to mention that it's already been available as an xpi for many months but
with little effect - because, of course, nobody used it which meant hardly any
sites support it, which means that the benefit for everyone is minimized. This
feature is only useful if it's widely adopted by websites, which will only
happen if it's widely available to users.

And I'd like to echo previous comments that this has been under design for 3
years and that you already *can* have it stay on or off with no content-area
resizing. Also, a fix to bug 103097 as is currently under discussion would
(almost) guarantee the toolbar appearing before any content ever does, which
would minimize this effect even further (you should never need a reflow due to
the toolbar appearing).

Comment 31

18 years ago
Regarding Simon Fraser's seccond point on 2001-10-04 11:39
I generally browse with the sidebar disabled. It would be annoying to have to
have it open jsut to use this feature.

At least with the toolbar I have it there ready to use when a site supports it.
I wouldn't even know if a site used <link> in a sidebar, without having the
sidebar open "Just incase", and that's not worth the space the sidebar takes.

Updated

18 years ago
Blocks: 103097
Note: I think binding XBL to <link> elements is a red herring. Firing events from
C++ code that bubble to JS code is a much safer and neater solution IMHO. One of
the problems of XBL on <link> is that authors can override it.
This bug being assigned to me might indicate to people that I plan to fix it.
This is not the case - I expect the Links toolbar community to pick up the torch
here. This is one of the high priorities for turther development of this
feature, so if you get this bugmail, please consider whether you are the person :-)

Reassigning bug to owner and QA contact of selected component.

Gerv
Assignee: gerv → pchen
QA Contact: claudius → sairuh

Comment 34

18 years ago
I'll take it
Assignee: pchen → jwbaker

Updated

18 years ago
Severity: major → normal
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.6

Comment 35

18 years ago
I've done a little work on this; I can't commit to anything because my time
availability is very sporadic, but if I do get a chance I'll post my results in
this bug. I have some preliminary code at home that's a long way from working,
that would also address bug 102990. But it has some bizarre results like
rendering all the page content, including the toolbar, in an approx 200x100
pixel area in the top left corner of the content area, and I never got past that
behavior onto the stuff that would really deal with this bug.
QA Contact: sairuh → claudius

Comment 36

18 years ago
I horked up a patch to fire the custom event but I don't THINK it works, because
adding a handler for DOMLinkInserted to linkToolBarOverlay.js did nothing.  I
hope one of you sharp fellows can set me straight.

Comment 38

18 years ago
Jeffrey, did you intend to attach this to bug 103097? That's the one that
requires custom events.
0.9.6 is out the door.


Updated

18 years ago
Keywords: mozilla0.9.7
Given that (if I read the status report right) the Links toolbar just got turned
off completely for a 3% startup perf gain, this bug just got a whole lot more
important.

Gerv
Severity: normal → blocker
Target Milestone: mozilla0.9.6 → mozilla0.9.7

Comment 41

18 years ago
Did hyatt's patch from 103097 get checked in or not?

Comment 42

18 years ago
Hyatt's patch is in, because my recent patch to bug 103097 relies on it and
works fine against the 0.9.6 release. If you plan on working on this bug, please
incorporate the changes in that patch, because it would be silly to take a 3%
new window improvement instead of a 5% pageload improvement.

Gerv, I think you may be misreading the status report - the following are the
parts that mention the sitenavbar:

http://mozilla.org/xpapps/progress/2001-11-16.html is the url for this status
report btw.

"* Bill Law shortened startup by 3% by removing links toolbar"

"Bill Law (law):
1. Measured "Cost of Chrome" for:
  a. entire status bar: removing this saved 3.7% of start up time
  b. links toolbar: removing this saved 2.94% of start up time (even though it
isn't used).
  c. status text/progressmeter: removing this saved only 0.36% (so it seems much
of the status bar cost is paid even though this portion of it is disabled)"
Since it seems unlikely that law removed the status bar or the progress meter,
and no bug numbers are quoted for removing it, I don't think it's been removed.
I also doubt that it could have got r/sr for removal this quickly without any of
us being aware of it until it was mentioned in a status report.

Can anyone confirm whether the linktoolbar (sorry sitenavbar) is in the latest
nightlies?

This bug is still urgent though. If I do get any chance to work on moz this
weekend, I'll work on this one rather than the other linktoolbar stuff I had
been planning on doing.

Comment 43

18 years ago
Yes, it's still there. It was a test. It was only a test. We now return you 
to your regularly scheduled programming.

Comment 44

18 years ago
I'm working on this bug, but XBL is fighting back and currently I'm losing ;)

If I ever get a patch that anywhere near works, I'll post it here. If anyone's
interested I can post the very incomplete and nonfunctional patch I have so far,
but I won't bother unless specifically requested because of it's horribly
nonworking state.

Comment 45

18 years ago
sballard took it.  obviously this is not a blocker and i'm moving it out one
more stone.
Assignee: jwbaker → sballard
Severity: blocker → normal
Status: ASSIGNED → NEW
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Since timeless needs something likse this for his project, I played around with
this and got a semi-working version. (currently it activates the link toolbar
only, no population).

Wish I had read this before, would have saved me some effort :) I currently use
hyatt's tabbrowser.xml <link> handling code. He checks if it was a faviicon, and
if not, dismissed it (return;). I simply added a method that then activates the
toolbar.

sballard: do you need this?  We also need to make the toolbar tab aware
(Assignee)

Comment 47

18 years ago
I think it would be better (read: cleaner) if you added your own link listener
instead of trying to reuse the tabbrowser.xml one. I believe tabbrowser nor
browser shouldn't have to know about the site navigation toolbar.

Comment 48

17 years ago
Progress report: I finally got a chance to work on this a little further. I now
have an xblified toolbar that loads on demand (ie on the first link-enabled
site) and displays the toolbar in the right circumstances, more-or-less, and
activates some of the buttons.

It's a thoroughly hacky job at this point: it relies heavily on
jssubscript-loader, and is designed to be minimally different from the old code.
Most of it needs significant reworking, quite apart from the fact that it
doesn't actually work once activated. In other words, it's not at a point to
even post the code here yet, let alone consider checking in.

Just thought that anybody interested in this bug might be interested to know
that some progress has been made on it. Still a long way to go, but this is a
significant milestone, IMHO at least :)

Comment 49

17 years ago
More progress: The toolbar actually *works* in some circumstances now, although
it's still a long way from working on in all circumstances. Major brokennesses are:
- The fullSlowRefresh function doesn't want to work, which is required on the
first page that uses the toolbar and also when switching it from "never" to any
other state. Thus it doesn't work the first time you visit a link-using page in
a new window, although it does approximately work after that.
- Only the top-level buttons work at the moment; the menus don't. This I know
how to fix, at least.

When I get both of these issues fixed I'll post a patch for testing. There are
also a bunch of other major issues:

- The code is based on the toolbar as it stood at 0.9.6. All fixes since then
need to be merged.
- The code is ugly as hell, mostly because for a first pass I wanted to make
minimal changes to the existing code. Once I have it working reliably, there are
a lot of cleanups I want to make.
- The code still relies a lot on document.getElementById("linktoolbar"). I'd
like to arrange for the code to be independent of whether it is bound to an
element with id of "linktoolbar" or not.

Anyway, at least progress is being made towards this bug :)

Comment 50

17 years ago
This shouldn't block a "page-loading perf issues" bug. The linktoolbar pageload
hit has already been addressed by bug 103097. This is a window-open and startup
perf hit only - is there a meta-bug for that?
No longer blocks: 71668

Comment 51

17 years ago
startup -> bug 7251
new window -> bug 49141

Comment 52

17 years ago
adding those to dependencies :)
Blocks: 7251, 49141

Comment 53

17 years ago
my guess this is not going to make mozilla0.9.8? So mozilla0.9.9 ?
Keywords: mozilla0.9.9

Updated

17 years ago
Keywords: mozilla1.0+

Comment 54

17 years ago
Unfortunately the way my time availability is turning out right now it seems
extremely unlikely that this will even make mozilla1.0. I'm setting the TM
accordingly based on what I personally will be able to do.

It's been suggested that if this can't be fixed for 1.0, the linktoolbar might
be backed out altogether. Nothing definite has been decided yet, but the
possibility is out there.

At some point this weekend I'll post the work I've begun on this. I haven't made
any progress since my last status report (Comment #49) so anyone who picks this
up will have a lot to do. But it might be necessary to do it if we want to keep
the toolbar for mozilla1.0 :(

If anyone does want to pick this up please comment in the bug or email me. I'll
be available to give tips or discuss my current implementation and why it's done
the way it is, and what I want to still accomplish. Perhaps some of the cleanup
issues can be deferred and, once fully working, a hacky version of this could be
landed on the 1.0 *branch* while we work towards a full clean version for the
trunk? I don't know.
Target Milestone: mozilla0.9.8 → mozilla1.1

Comment 55

17 years ago
Okay, here goes with posting my work-in-progress. If anyone is interested in
working on this, please be aware of the contents of comment #49 on this bug, and
also post here before starting in case I can think of any pointers to give.

I'm going to attach three files: The first is a diff, and the other two are new
files. I've been working on this with patchmaker and so the locations for the
new files are given in terms of location in the "chrome" directory of a finished
build. They are content/navigator/linkToolbarUI.js (a replacement for
linkToolbarOverlay.js) and content/navigator/linkToolbarBindings.xml (the XBL
implementation itself).

The most important thing to be aware of is that all linktoolbar fixes since
0.9.6 still need to be backported into this codebase, because the majority of
the code is moved around into new files which won't automatically patch.

Oh, and I'm adding choess to the cc list in a vague attempt to make sure that
anyone who might possibly be interested in helping out here is at least aware of
the issues.

Okay, patch and new files coming up...

Updated

17 years ago
Attachment #73405 - Flags: needs-work+

Updated

17 years ago
Attachment #73406 - Flags: needs-work+

Updated

17 years ago
Attachment #73407 - Flags: needs-work+
Keywords: mozilla0.9.7, mozilla0.9.9

Comment 59

17 years ago
If this doesn't work by Wednesday someone might back out the links toolbar and
that will really ruin my day.
Assignee: sballard → jwbaker

Updated

17 years ago
Priority: -- → P1
Target Milestone: mozilla1.1alpha → mozilla1.0

Comment 60

17 years ago
I actually don't want to get involved in this again.  Drivers don't want this
for 1.0.  Let them drop it for a few milliseconds in new window performance. 
Galeon already has a links toolbar.  Why should I care.
Assignee: jwbaker → sballard
Priority: P1 → --
Target Milestone: mozilla1.0 → Future
> Drivers don't want this for 1.0.

Where did you get that idea? Have there been mails I haven't seen?

drivers@mozilla.org will assess any patch, on its merits, for 1.0. If you can
fix the perf problems with a one-liner, the links toolbar is in. If it requires
a total rewrite, it's probably out. But, in between, we can't tell until you
produce a patch.

The closer the patch is to 1.0, the smaller and lower-risk it has to be. So why
not make one ASAP and try your luck? If it doesn't get in, fine - we check it in
after we branch.

Gerv

Comment 62

17 years ago
The patch applies with a bit of fuzz, but the toolbar doesn't work at all. 
"linkToolbarUI is undefined", sayeth STDERR.

Comment 63

17 years ago
Hmm, it worked (ish) for me last time I tested it, but that was a while ago.
linkToolbarUI is (as you might guess) defined and set up in linkToolbarUI.js
which is (should be) included from linkToolbarOverlay.xul. Perhaps this will
give you enough of a start on debugging?

One thing you could try is commenting out the fullSlowRefresh() call in the
Bindings.xml <constructor> - this is theoretically needed, but when I was
testing it caused problems. Without that call, the toolbar would fail to show up
for the first page that used it, but work fine for the second and subsequent
pages.

The patch right now only supports toolbar buttons, not menus; to fix that you
need to change the code that populates the variable "hooks" in
LinkToolbarHandler.createItemForLinkType. This needs to recursively descend the
node tree rather than just looping across the variable "nodes".

I wish I could offer to provide real-time debugging assistance, but it wouldn't
be fair to make any such commitment right now because I probably can't honor it.
I'll certainly try to provide answers in as close to realtime as I possibly can
in bugzilla, and if I ever do get any time availability in the evenings I'll
make myself available on IRC. If there's any help I can give in explaining how
the patch is supposed to be working, let me know either here or by email.

Comment 64

17 years ago
I've filed bug 138496 to back out the linktoolbar as a result of this bug not
getting fixed for 1.0. I don't like doing this, but it seems inevitable and I
think it would be wrong of me to let the linktoolbar stay in just because all of
the drivers forgot about it, when I knew they wanted it backed out and didn't
forget about it.

If you don't like this, discuss alternatives in bug 138496 or come up with a
miracle fix to this bug for the 1.0 timeframe.

Comment 65

17 years ago
Patch exists, but needs a little work.

What remains to be done in this bug? Shouldn't we open separate issues for all
the separate issues that remain? Can you sketch out in pseduocode what remains
to be done?

I strongly vote in favor of fixing this for 1.0. This is the API freeze release.
We need the links toolbar as a key part of our arsenal of features that the
competition doesn't have. 

It's Mozilla1.0+. Why isn't the target milestone set at 1.0?

This is major severity.
Severity: normal → major
Keywords: nsbeta1, patch, topembed
topembed? you got to be kidding me.

Comment 67

17 years ago
Yeah. Didn't mean to include topembed. Sorry, and thanks for catching that. 
Keywords: topembed

Updated

17 years ago
Keywords: nsbeta1, patch

Comment 68

17 years ago
It has a patch that *doesn't work*. The patch keyword is inappropriate.

It has nothing to do with embedding.

And Netscape would be crazy to even think about including it in anything like
it's current state.

The current patch needs probably a workday's worth of solid hacking to make it
minimally usable (ie "as good as the current state of the code"). If you want to
get this bug fixed, HELP FIX IT. Don't try to magic problems away by setting
inappropriate keywords on bugs.

A better solution if you really care about 1.0 would be to investigate the XPI
solution. That's something that's actually achievable if a single person with
moderate knowledge was willing to spend a few hours hacking at it, and a bit of
politicking to get it checked in. But needless to say, throwing keywords around
and yelling is much easier.

Comment 69

17 years ago
Oh, and it's mozilla1.0+ because drivers have indicated they'd like to see this
fixed for mozilla1.0. That doesn't mean that somebody's magically going to turn
up and actually do the work to fix it. As was made clear in comments 54 through
60, I don't have the time to do that, and I made it very clear that I hoped
somebody else would step up. Nobody did.

It's not up to drivers to force people to fix bugs. It's up to the *people who
care about those features*. I care about this feature enough to have worked on
it numerous times (I saved it from the initial backout way back in 0.9.5 by
helping to eliminate the pageload perf hit when the toolbar was disabled, and
wrote the fix for bug 103097 to eliminate the pageload hit the rest of the time,
and I wrote the patch in this bug). Regardless, I recognize that *I have no
right to complain* that it's being backed out, because I wasn't able to do
enough to solve all the problems that were considered vital.

So don't complain at me. And don't bitch in my bugs. Either FIX THEM, or find
somebody who has the time and skills and persuade or bribe them to fix them. I'm
as upset as you are, but *please*, use your righteous indignation productively
or not at all, because bugs with too high a noise-to-signal ratio are almost
guaranteed to get ignored. 

Comment 70

17 years ago
Renominating for nsbeta1. This bug is not currently assigned. Maybe someone in
Netscape could make a contribution to the bug. Admittedly, that might push the
fix back to 1.0.1, but that's all right. I can't help out personally, because I
do not understand how to fix the bug, and I don't have coding skills.

I find it a little baffling when people ask for comments on whether a bug should
be fixed or a feature backed out, and then criticize someone who says he wants
the bug to be fixed.
Keywords: nsbeta1

Comment 71

17 years ago
I don't think I asked for opinions on whether fixing this bug was a good idea,
either here or in bug 138496. *Obviously*, fixing the bug would be preferable to
backing out the feature. What I said was:

"If you don't like this, discuss alternatives in bug 138496 or come up with a
miracle fix to this bug for the 1.0 timeframe."

And over in bug 138496 I said:

"If you have a problem with this, don't complain to me or to drivers, either fix
bug 102992 or come up with the changes to make this an optional XPI."

Nobody *wants* to see the toolbar backed out. Everyone would love to see this
bug fixed. If that can be done, great (Netscape don't care about the linktoolbar
feature, and sometimes seem to be actively opposed to it - they're unlikely to
commit a programmer to fix it). You can leave the nsbeta1 nomination I guess,
but having seen Netscape's reaction to several linktoolbar related bugs, I feel
safe in saying it'll make no difference.

Discussing realistic alternatives is fine (probably should be done in bug 138496
rather than here, though, unless the alternative involves converting the
linktoolbar to XBL). Simply saying that you don't want the world to be the way
the world clearly *is*, is redundant.

Sorry if you interpreted my earlier statements as requesting that kind of
comment. And sorry if I was harsh - I'm as pissed off that this is happening as
anyone could possibly be, as I've invested quite a lot of time into the toolbar,
and that's probably making me snappy. Sorry.

Comment 72

17 years ago
Is this bug dependent on bug 119087?

Comment 73

17 years ago
Nope. It will take advantage of it automatically when it's done, just as the
current code does, but it can be implemented to still be a vast improvement over
the current code even without it.

Updated

17 years ago
Blocks: 143727

Updated

17 years ago
Keywords: mozilla1.1
adt: nsbeta1-
Keywords: nsbeta1 → nsbeta1-

Comment 75

15 years ago
Just wondering the latest status on this one.
The status is that someone needs to do it.  No one has been found so far who's
willing to.
Keywords: helpwanted
Product: Core → Mozilla Application Suite
Assignee: sballard → jag
QA Contact: claudius
Target Milestone: Future → ---

Comment 77

10 years ago
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614
Status: NEW → UNCONFIRMED

Comment 78

9 years ago
*** This bug has been confirmed by popular vote. ***
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 79

8 years ago
The SeaMonkey team will not be devoting any resources to this. If you want to work on this bug and drive a patch into the tree please open a NEW bug.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.