Last Comment Bug 467045 - Custom Buttons2 extension broken after Bug 407725
: Custom Buttons2 extension broken after Bug 407725
Status: RESOLVED FIXED
: dataloss, dev-doc-complete, regression, relnote
Product: Toolkit
Classification: Components
Component: Toolbars and Toolbar Customization (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks: 407725
  Show dependency treegraph
 
Reported: 2008-11-27 16:09 PST by Ria Klaassen (not reading all bugmail)
Modified: 2009-02-15 18:11 PST (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
test profile (493.25 KB, application/zip)
2008-11-27 16:41 PST, Ria Klaassen (not reading all bugmail)
no flags Details

Description Ria Klaassen (not reading all bugmail) 2008-11-27 16:09:54 PST
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) 2008-11-27 16:37:04 PST
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) 2008-11-27 16:41:54 PST
Created attachment 350409 [details]
test profile

A bit smaller.
Comment 3 :Gavin Sharp [email: gavin@gavinsharp.com] 2008-11-27 19:08:17 PST
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 2008-11-27 23:10:51 PST
I am contacting George to see if he has any input on this.  Please stay tuned.
Comment 5 Philip Chee 2008-11-28 00:04:30 PST
> 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 2008-11-28 04:58:46 PST
> 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 2008-11-30 11:41:57 PST
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 2008-11-30 11:46:14 PST
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) 2008-11-30 12:56:19 PST
Great, thanks, I'll retest as soon as bug 407725 is checked in again.
Comment 10 Ria Klaassen (not reading all bugmail) 2008-12-03 06:04:28 PST
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 2008-12-18 02:59:43 PST
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] 2009-02-12 00:52:33 PST
What exactly needs to be documented here?
Comment 13 Philip Chee 2009-02-12 01:53:14 PST
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] 2009-02-12 08:20:18 PST
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 2009-02-12 09:41:45 PST
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] 2009-02-12 10:30:47 PST
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] 2009-02-12 10:33:40 PST
From the palette that is.
Comment 18 :Gavin Sharp [email: gavin@gavinsharp.com] 2009-02-12 10:56:44 PST
(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] 2009-02-15 18:11:50 PST
I've added a note to:

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

That should round out the doc needs for this issue.

Note You need to log in before you can comment on or make changes to this bug.