467045 - Custom Buttons2 extension broken after Bug 407725

Status: RESOLVED FIXED

(Toolkit :: Toolbars and Toolbar Customization, defect)

Description

[Ria Klaassen (not reading all bugmail)]

After every restart all Custom Buttons disappear. If I replace the two files buttonsoverlay.xul and buttonsoverlay.bak with a backup, they appear again (for one session).
I'll attach a test profile with extension and files.

Comment 1

[Ria Klaassen (not reading all bugmail)]

The attachment is too large for bugzilla, so if someone wants to investigate this, I can send it by e-mail.

Comment 2

[Ria Klaassen (not reading all bugmail)]

Attachment: test profile

A bit smaller.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

The fundamental change from bug 407725 is that toolbar items are no longer cloned from the palette, and are instead moved from the palette to the toolbar.

Nicholas/Jeff/George: can any of you comment on what it would take to get custom buttons 2 updated to deal with these changes?

Comment 4

[Nicholas Alipaz]

I am contacting George to see if he has any input on this.  Please stay tuned.

Comment 5

[Philip Chee]

> The fundamental change from bug 407725 is that toolbar items are no longer
> cloned from the palette, and are instead moved from the palette to the toolbar.

When Custom Buttons2 saves its settings, it looks through the .palette (gToolbox.palette.childNodes) for any custom buttons defined. If some or all of the buttons are on a toolbar, the code doesn't find these and when their overlay.xul is created it doesn't contain any of the buttons that are active on a toolbar.

It's just a SMOP to look at the nodes in the toolbars as well e.g.

var buts=gToolbox.getElementsByAttribute("id", "*")
for (var j=0;j<buts.length;j++){
  var but=buts[j];
  if (but.getAttribute("id").indexOf("custombuttons-button")!=-1){     // Our Kind of button
  .....etc....
  }
}

Plus, they'll have to filter for duplicates for Gecko < 1.9.1b2

Comment 6

[Philip Chee]

> It's just a SMOP to look at the nodes in the toolbars as well e.g.

> var buts=gToolbox.getElementsByAttribute("id", "*")
> for (var j=0;j<buts.length;j++){
...

Ayup. Just tested. Adding this to the CB2 code prevents the CB2 buttons from disappearing.

Comment 7

[Nicholas Alipaz]

Thanks for the code and the testing Phil.  I have added the fix to a prerelease of the FF3.1 version of the extension.
http://custombuttons2.com/extension/custom-buttons2-2-0-8.5.html

Let us know if you have any issues with it.

Comment 8

[Nicholas Alipaz]

slight error in the url I posted, here is the corrected one:
http://custombuttons2.com/extension/custom-buttons2-2-0-8-5.html

Comment 9

[Ria Klaassen (not reading all bugmail)]

Great, thanks, I'll retest as soon as bug 407725 is checked in again.

Comment 10

[Ria Klaassen (not reading all bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20081203 Minefield/3.2a1pre

Bug 407725 is checked in and I just verified that the updated extension fixes the issue (assuming that bug 407725 still contains the CB2 breaking code).

Comment 11

[Nicholas Alipaz]

Please have a look at the latest version:
http://custombuttons2.com/extension/beta-version-2-0-7-101b-01-released.html

to fix issues in shireteko 3.1b3pre

Comment 12

[Eric Shepherd [:sheppy]]

What exactly needs to be documented here?

Comment 13

[Philip Chee]

Previously the <toolbox>.palette held a complete set of customizable buttons (and toolbaritems) . Any buttons appearing on a toolbar were cloned from that set. After this patch, buttons are not cloned but moved to/from the .palette. This means that now:
1. The .palette does not hold a full set of customizable buttons.
2. Only one copy of any button exists at any time.

Additionally while Firefox (and Thunderbird) continues to remove the <toolbarpalette> element from the DOM (the .palette property holds a continuing reference to it); the new implementation in SeaMonkey 2.0a does not remove it from the DOM.

Comment 14

[George Dunham [SCClockDr]]

RE Comment 13
Buttons are now handled correctly in Custom Buttons² from Version 2.0.7.100 2008-12-13 in:
1. Firefox versions 2.0.0.19,20, 3.0.4,6, 3.1b3pre, 3.2a1pre
2. Thunderbird versions 2.0.0.19, 3.0b2pre
Custom Buttons²now gathers all active buttons from the toolbars and inactive buttons from the palette as would be expected.

I also note this bug's status is RESOLVED FIXED!

I am unclear what the following from comment 13 intends to mean:
"...Additionally while Firefox (and Thunderbird) continues to remove the
<toolbarpalette> element from the DOM (the .palette property holds a continuing
reference to it); the new implementation in SeaMonkey 2.0a does not remove it
from the DOM."

Comment 15

[Philip Chee]

Sheppy is the DevMo guy in charge of updating the documentation. He was responding to the "dev-doc-needed" keyword.

> I am unclear what the following from comment 13 intends to mean:

In the standard implementation (e.g. Firefox), the constructor of the toolkit toolbar binding removes the <toolbarpalette> from the DOM. The SeaMonkey derived toolbar binding doesn't do that.

From the extension authors point of view, doing a |document.getElementById("buttonID")| will work in SeaMonkey even if the button isn't on a toolbar. Compare this to Firefox where this works only when the button is placed on a toolbar.

Comment 16

[George Dunham [SCClockDr]]

Thanks Philip

That clarifies why I need to use |but = this.gToolbox.palette.childNodes[ j ];| in a loop to dig the inactive button nodes from the palette.

Seems to me (age in senior moment territory) |document.getElementById("buttonID")| has not yielded a node for some time in Fx or Tb.

Comment 17

[George Dunham [SCClockDr]]

From the palette that is.

Comment 18

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #12)
> What exactly needs to be documented here?

I've already written something up on devmo about this change, for what it's worth:

https://developer.mozilla.org/En/Updating_extensions_for_Firefox_3.1#Customizable_toolbars

I think it's pretty consistent with Philip's summary in comment 13.

Comment 19

[Eric Shepherd [:sheppy]]

I've added a note to:

https://developer.mozilla.org/En/XUL/Toolbar

That should round out the doc needs for this issue.