Closed Bug 658444 Opened 13 years ago Closed 13 years ago

installing Firebug 1.7.1 removes contents of Navigation Toolbar

Categories

(SeaMonkey :: General, defect)

SeaMonkey 2.1 Branch
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: info, Unassigned)

References

()

Details

(Whiteboard: [SmBugEvent])

User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0.2pre) Gecko/20110517 Firefox/4.0.2pre SeaMonkey/2.1.1pre
Build Identifier: Mozilla/5.0 (X11; Linux x86_64; rv:2.0.2pre) Gecko/20110511 Firefox/4.0.2pre SeaMonkey/2.1pre

I updated to SeaMonkey 2.1.1pre nightly and installed Firebug 1.7.1, and my Navigation Toolbar contents went away; only a Firebug icon and drop-down menu appeared.  I also reproduced using 2.1 RC.

Reproducible: Always

Steps to Reproduce:
1. Go to http://blog.getfirebug.com/2011/05/13/firebug-1-7-1/2.
2. Click the link "getfirebug.com has Firebug 1.7.1" , allow install.
3. Restart the browser.
4. Disable Firebug and restart the browser.
5. Remove the Firebug extension and restart the browser.

Actual Results:  
The contents of the Navigation Toolbar -- back/forward buttons, location field, search box, etc. -- disappear, replaced only by the Firebug icon!

Disabling the add-on made Navigation Toolbar completely empty.  Removing the add-on altogether didn't bring them back (!).  I have to restart in Safe Mode or with a new clean profile

Expected Results:  
Don't lose the Navigation Bar contents.

This is probably a bug in Firebug; I'm filing against SeaMonkey in case there's something about 2.1 that makes it more likely that extensions can nuke the Navigation Toolbar.  I'm guessing that Firebug is trying to add itself to the Add-on toolbar that replaces the status bar in Firefox 4, but somehow SeaMonkey's toolbars confuse it.  I don't know why removing the add-on altogether doesn't fix the problem.  At least this issue with Firebug 1.7.1 should be be noted in the SeaMonkey 2.1 Release Notes?
Keywords: relnote
Version: unspecified → SeaMonkey 2.1 Branch
I restored my Navigation Toolbar contents by quitting SeaMonkey, then editing localstore.rdf and deleting a tag that included
 <RDF:Description RDF:about="chrome://navigator/content/navigator.xul#nav-bar"
  ... 
  currentset="window-controls,firebug-button"
  ... />
I think you should file a bug in the firebug issue tracker.
Whiteboard: [SmBugEvent]
I think you better talk to the SeaMonkey folks. All we do is set the install.rdf for them. We don't use SeaMonkey and we don't even know what it is. But we'd be glad to help anyone from the SeaMonkey team solve this.
Sounds like FB issue 4086. I haven't checked whether/when this regressed yet.
Confirming (on Linux). Strange, FB issue 4086 was supposed to be fixed in FB 1.7a11 but even with that version and a current SM 2.1 branch nightly the bug appears. Maybe SM or the platform changed meanwhile. Oh well.
Status: UNCONFIRMED → NEW
Ever confirmed: true
After some analysis:

1) The currentset attribute is not present initially but only after adding or removing buttons from the toolbar. I guess when I tested last time (issue 4086) I was in this state. If you do this with a new profile and only then install FB, this bug does not appear.

2) When FB's start-button code executes (method appendToToolbar), SeaMonkey seems to not have finished setting up the toolbar yet so the currentSet property only contains "window-controls". If point (1) from above applies, the currentset attribute contains the correct list so the code does the right thing. Otherwise this bug happens.

So to sum it up, the fix for issue 4086 was wrong because my analysis back then was incomplete. Instead, the appendToToolbar method call must be delayed until the toolbar has been set up (using setTimeout and friends).

If the above is too complicated, another solution would be to not call the above method at all for SeaMonkey, since we still have the status bar and the icon there.
I'd expect Firebug to do all their stuff in a window onload listener. Don't they?
(In reply to comment #7)
> I'd expect Firebug to do all their stuff in a window onload listener. Don't
> they?
I just tried to append the button in onload event handler and it seems to be too late. The button doesn't appear and I need to restart SeaMonkey (or Firefox) to see it. This is bad since the button must be visible immediately after the extension is installed and browser restarted once.


(In reply to comment #6)
> If the above is too complicated, another solution would be to not call the
> above method at all for SeaMonkey, since we still have the status bar and
> the icon there.
Yes, I could disable the feature code for SeaMonkey. I guess I should use nsIXULAppInfo, correct?

But if there is a working way how to properly append that button into SeaMonkey toolbar I am happy to do that? Any tips?

Honza
(In reply to comment #9)
> I just tried to append the button in onload event handler and it seems to be
> too late.

Then maybe try setTimeout(appendToToolbar, 0). Just guessing.

> Yes, I could disable the feature code for SeaMonkey. I guess I should use
> nsIXULAppInfo, correct?

Yes. I just updated the MDC pages on the topic which didn't cover SM until now. You can either use the ID or check the name (in which case you'd be careful to use "SeaMonkey" and not "Seamonkey").

> But if there is a working way how to properly append that button into
> SeaMonkey toolbar I am happy to do that? Any tips?

If only I knew. CCing Philip (although he already seems to watch this) and Neil. If anyone from our side knows, then him. ;-)

I'm with you here. If we can get it fixed properly we should try hard. But if we can't, especially in time for Firebug 1.8, then please do the quick fix.

BTW: A few lines below the appendToToolbar call you're checking the app version number. You should change that check to only apply to Firefox (again, ID or name check works).
Jens, I think your appendToToolbar() code is wrong. The following code should work for both Firefox and SeaMonkey (we use the same toolbar bindings - module some XBL extends).

document.getElementById("nav-bar").insertItem("smxtra-button");
var currentset = document.getElementById("nav-bar").currentSet;
document.getElementById("nav-bar").setAttribute("currentset",currentset);
document.persist("nav-bar","currentset");

> I just tried to append the button in onload event handler and it seems to be too late.
This code should work from the onLoad handler or at any time between onload and onclose.
(In reply to comment #11)
> document.getElementById("nav-bar").insertItem("smxtra-button");
> var currentset = document.getElementById("nav-bar").currentSet;
> document.getElementById("nav-bar").setAttribute("currentset",currentset);
> document.persist("nav-bar","currentset");
Yep, works now, thanks!

Related commit info:
http://code.google.com/p/fbug/issues/detail?id=4086#c12

I tested with Firefox 5 and SeaMonkey 2.1 and works for me

Will be part of Firebug 1.8a3
Somebody should verify as soon as 1.8a3 is out

Thanks!
Honza
(In reply to comment #11)
> Jens, I think your appendToToolbar() code is wrong.

I already admitted that my approach was wrong, but to be fair, it's not really my code for the most part. ;-)

(In reply to comment #12)
> I tested with Firefox 5 and SeaMonkey 2.1 and works for me

Maybe I made a mistake applying the changes to FB 1.7.1 but for me the Firebug button is not added (only tested with SM 2.1 so far). The toolbar doesn't break anymore either, though.

> Will be part of Firebug 1.8a3
> Somebody should verify as soon as 1.8a3 is out

Will do. ETA?

TUVM!
http://code.google.com/p/fbug/source/detail?r=10600
> if (appInfo.name == "Firefox" && versionChecker.compare(appInfo.version, "4.0*") >= 0)
I think appInfo.id would be better since Fennec/Firefox mobile identifies itself as "Firefox" as well. Of course Firebug doesn't work in Mobile so the point is moot.
(In reply to comment #13)
> Maybe I made a mistake applying the changes to FB 1.7.1 but for me the
> Firebug button is not added (only tested with SM 2.1 so far). The toolbar
> doesn't break anymore either, though.

Some further investigation showed that the new method of calling appendToToolbar doesn't work in FB 1.7.1 as-is, so the function wasn't called at all. No wonder the toolbar was OK! ;-)

Anyway, I just grabbed FB from SVN, ran "ant" in branches/firebug1.8 and installed the resulting XPI from release/ (which is still called firebug-1.8.0a2.xpi BTW). Both this bug (issue 4086) and the detaching problem (issue 2498) have been fixed. Hooray! :-)

When 1.8.0a3 proper is released I'll verify again and close this bug. For the time being we're done here since I already relnoted this (see comment 8).
(In reply to comment #13)
> Will do. ETA?
Firebug 1.8 goes to beta in 1-2 weeks. Final version will be available as soon as Firebug 5 is out (at the same time).

(In reply to comment #14)
> Some further investigation showed that the new method of calling
> appendToToolbar doesn't work in FB 1.7.1 as-is, so the function wasn't
> called at all. No wonder the toolbar was OK! ;-)
Yes, Firebug 1.7 needs slightly different patch. I'll port it back only if we would plan another 1.7 update (but 1.8 will be out soon).

> Anyway, I just grabbed FB from SVN, ran "ant" in branches/firebug1.8 and
> installed the resulting XPI from release/ (which is still called
> firebug-1.8.0a2.xpi BTW). Both this bug (issue 4086) and the detaching
> problem (issue 2498) have been fixed. Hooray! :-)
Great!

> When 1.8.0a3 proper is released I'll verify again and close this bug. For
> the time being we're done here since I already relnoted this (see comment 8).
Thanks!

Please cc me on any bug that is related to Firebug and I'll investigate/fix.

Honza
Verified with FB 1.8.0a3 and SM 2.1.1pre. TUVM!
Status: NEW → RESOLVED
Closed: 13 years ago
OS: Linux → All
Hardware: x86_64 → All
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
FTR: This will likely not be fixed for Firebug 1.7.x but only Firebug 1.8.x.
(In reply to Jens Hatlak (:InvisibleSmiley) from comment #18)
> FTR: This will likely not be fixed for Firebug 1.7.x but only Firebug 1.8.x.
That's correct, nobody is working on 1.7.x anymore.
Honza
You need to log in before you can comment on or make changes to this bug.