101131 - PT Bookmarks menu stops working after installing an XPI add on

Status: RESOLVED FIXED

(Core :: XUL, defect, P3)

Description

[Jason Kersey]

Open the browser, and click the PT bookmarks button.  I get the top three
hardcoded menuitems, add, file, manage, and that's it.  Is this from the
tabbrowser checkin?  The menubar menu works.

Comment 1

[Matthias Versen [:Matti]]

wfm with win2k and a 2h old CVS build...
BTW:Stupid bugzilla newbie :Please include in all your bug reports the build ID. :-)

Comment 2

[Mike Young]

I saw this too, but it went away. It was in the 20010920 or 19 build. Now in
20010921 I can't get it to happen again. 

I was on Win98

Comment 3

[David Hyatt]

Button works fine for me in current builds.

Comment 4

[Jason Kersey]

I see this in 2001092208.  Existing profile.  blake said this may be hewitt.  to 
him to look at.  I get:

Error: this.docShell has no properties
Source File: 
chrome://global/content/bindings/browser.xml#browser.webNavigation (getter)
Line: 0

in the js console, unsure if this is related.

Comment 5

[Jason Kersey]

->hewitt

Comment 6

[Blake Ross]

I'm with Mike, I saw this but then it went away (i think with a few restarts,
not with a new build).

Comment 7

[Mike Young]

I'm seeing this again (right now) build 2001092103

I've closed and reopened, restarted the computer, mozilla and opened new windows.
No Luck. Still can't get it to work.

attaching screenshot.

Comment 8

[Mike Young]

Attachment: screenshot

Comment 9

[John Morrison]

see also bug 101239, where the menu popup won't rollup after first shown.

Comment 10

[Joe Hewitt (gone)]

works for me

Comment 11

[Jason Kersey]

come to my cube and see it.  it still doesn't work in a new profile.

Comment 12

[Joe Hewitt (gone)]

oh, wait, I misunderstood the problem... I do indeed see it now.

Comment 13

[Mike Young]

I don't know how or why, but I think I can reproduce this by installing an XPI.
I realized that this went away with new builds and then resurfaced sometimes
after I had been working, and I believe its when I install something in Mozilla.
I've installed the Preferences toolbar and Mozgest, which all seem to cause this
problem. You can get the Preferences toolbar at
http://www.xulplanet.com/downloads/view.cgi?category=applications&view=all and
try to see whether you reproduce it.

Comment 14

[Brett Denny]

Yes!! I did notice this too awhile back but until now had forgotten it.  I have
the preferences toolbar installed too - not sure if that was what had done it
but I can't think of anything else offhand that I've installed.  I suspect
uninstalling it or more likely doing a fresh install will solve the problem but
I haven't tried it yet... at least not in a few weeks.

Comment 15

[Mike Young]

I just performed a fresh install of the latest nightly, and the menu worked
fine. I installed the preferences toolbar, and the menu no longer worked. I
reinstalled mozilla, and the bar works again. So whether installing an XPI is
the only cause, I don't know, but it *is* one cause.

Comment 16

[Gervase Markham [:gerv]]

I'm getting this as well. Feel free to drop by to see.

Gerv

Comment 17

[Joe Hewitt (gone)]

I did some testing and discovered the problem is that the personal toolbar
bookmarks button references its template using the template attribute, rather
than a template child.  The template it uses, bookmarksMenuTemplate, comes from
navigatorOverlay.xul.  Apparently, the xul content builder is looking for the
template before it is inserted by the overlay.  This doesn't always happen to
me, so it must be a timing issue.

I was able to solve the problem by moving the template directly into
navigator.xul.  I'm going to re-assign this bug to Ben because this is really
his domain, and I'll let him decide where to put the template.

Comment 18

[Ben Goodger (use ben at mozilla dot org for email)]

investigating....

Comment 19

[Ben Goodger (use ben at mozilla dot org for email)]

OK. Here's a patch that fixes the symptom and should get the menu running 
again. I couldn't reproduce this bug on any of my builds so I can't verify that 
it actually works, please let me know if it doesn't, I can think of several 
alternatives. 

This doesn't fix the problem properly. A proposal for a better solution is 
described in a comment in this patch. If I can get approval to investigate that 
route, it should be followed and this code removed/ignored. 

Comment 20

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: quick-fix.

Comment 21

[Ben Goodger (use ben at mozilla dot org for email)]

The whitespace issue in the attached patch has been corrected. 

Comment 22

[Jason Kersey]

Comment on attachment 51852 [details] [diff] [review]
quick-fix.

r=kerz

Comment 23

[Joe Hewitt (gone)]

Comment on attachment 51852 [details] [diff] [review]
quick-fix.

It's yucky, but it should work. sr=hewitt

Comment 24

[Asa Dotzler [:asa]]

Comment on attachment 51852 [details] [diff] [review]
quick-fix.

a=asa (on behalf of drivers) for checkin to 0.9.5. hurry.

Comment 25

[Brett Denny]

Interesting.... when running Mozilla by itself (Build 2001100503) I see this
bug, no menu - BUT - when running Multizilla (v1.0.99E) on this build, the
bookmarks menu appears correctly.

Comment 26

[Ben Goodger (use ben at mozilla dot org for email)]

Fixed checked in, branch & trunk,

Comment 27

[Blake Ross]

Er...does anyone know what suddenly caused this, though?   The bookmarks button 
has been referencing a remote template by id forever.  If it was caused by 
hewitt's toolbarbutton changes, and I think it was, it'd be worth looking into 
why.

Comment 28

[Ben Goodger (use ben at mozilla dot org for email)]

Hewitt had toolbarbutton changes?

What did they involve?

Comment 29

[Ben Goodger (use ben at mozilla dot org for email)]

HJ says this still isn't working right. 

Comment 30

[HJ]

Correct, the first time it doesn't open/activate the bookmark popup.

WinNT4 with the build 10-08

Comment 31

[[not reading bugmail]]

I found that I think its the preferences bar that does it  (remember that it is
0.9.3 code and works on those builds but not properly on new ones), and if you
then install Multizilla you get it working while using Multizilla but not
Mozilla itself.. 

It you totally delete mozilla and your profile, and start a new install from
scratch this bug doesn't return.

/blame Pref Toolbar causes the problems not Mozilla ;)

-dman84

Comment 32

[HJ]

What is 0.9.3 code, MultiZilla? Preference toolbar? Which preference toolbar are
you talking about? 

And FYI, MultiZilla is currently based on mozilla 0.9.4+ (builds after sept/30)
so no 0.9.3 there's no 0.9.3 code here and we are using 0.9.5 sources at this
moment.

0.9.3. man this guy is nuts.

Comment 33

[Jason Kersey]

This is definately either the mozgest toolbar or the UA toolbar for me. 
Resummarizing.

Comment 34

[Asa Dotzler [:asa]]

0.9.5 is out the door. bumping TM up by one.

Comment 35

[Ben Goodger (use ben at mozilla dot org for email)]

Bumping down to P3, since the problem appears to be related to an unsupported
add-on. 

I'll remove the hacky code asap. 

Comment 36

[Pavol Vaskovic]

Ben said:
< ... related to an unsupported add-on. 

You are talking about mozgest or Multizilla? I mean, which on e is causing
problem and why. I'd be glad to correct this if it is in our part. Any
suggestion are appreciated.
--mondo (working on mozgest)

Comment 37

[Ben Goodger (use ben at mozilla dot org for email)]

My comments were based on other comments further up where people claimed add-on
components were what "did it".. see cuz84d's comments

Comment 38

[Pavol Vaskovic]

Ben: I'm not sure... Maybe I don't see things from the right angle, but IMHO
those ad-ons did not cause this bug. They just made it visible. (if I understood
corectly what was the problem here)
--mondo

Comment 39

[Henrik Gemal]

the javascript error described in this bug is covered in bug 104360

Comment 40

[Asa Dotzler [:asa]]

Comment on attachment 51852 [details] [diff] [review]
quick-fix.

obsoleting old checked in patches

Comment 41

[Neil Pryde]

This patch should go in again for builds 20030325 and up. I had to e-mail HJ
about this bug and he told me to insert this removed code for MultiZilla and now
the bookmarks button works again.

http://lxr.mozilla.org/seamonkey/source/xpfe/browser/resources/content/navigator.xul#271
-         <menupopup
onpopupshowing="updateGroupmarkMenuitem('PT_bookmarks_groupmark');">
+         <menupopup onpopupshowing="checkBookmarksMenuTemplateBuilder();
updateGroupmarkMenuitem('PT_bookmarks_groupmark');">

http://lxr.mozilla.org/mozilla/source/xpfe/components/bookmarks/resources/bookmarksMenu.js#805
+// An interim workaround for 101131 - Bookmarks Toolbar button nonfunctional.
+// This simply checks to see if the bookmark menu is empty (aside from static
+// items) when it is opened and if it is, prompts a rebuild. 
+// The best fix for this is more time consuming, and relies on document
+// <template>s without content (referencing a remote <template/> by id) 
+// be noted as 'waiting' for a template to load from somewhere. When the 
+// ::Merge function in nsXULDocument is called and a template node inserted, 
+// the id of the template to be inserted is looked up in the map of waiting
+// references, and then the template builder hooked up. 
+function checkBookmarksMenuTemplateBuilder()
+{
+  var lastStaticSeparator = document.getElementById("lastStaticSeparator");
+  if (!lastStaticSeparator.nextSibling) {
+    var button = document.getElementById("bookmarks-button");
+    button.builder.rebuild();
+  }
+}

-Neil.

Comment 42

[Jan Varga [:janv]]

-> me

Comment 43

[Jan Varga [:janv]]

fixed along with bug 199178