Last Comment Bug 608687 - <rule>-generated toolbar items are cloned when leaving toolbar customize
: <rule>-generated toolbar items are cloned when leaving toolbar customize
Status: VERIFIED FIXED
: regression, verified1.9.1, verified1.9.2
Product: Toolkit
Classification: Components
Component: Toolbars and Toolbar Customization (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla2.0b8
Assigned To: Neil Deakin
:
Mentors:
https://www.mozdev.org/bugs/show_bug....
Depends on:
Blocks: 553808 596232
  Show dependency treegraph
 
Reported: 2010-11-01 03:43 PDT by Manuel Reimer
Modified: 2011-01-24 15:26 PST (History)
13 users (show)
enndeakin: in‑testsuite+
hskupin: in‑litmus-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
betaN+
.14+
.14-fixed
.17+
.17-fixed


Attachments
Clear mRoot after calling Uninit (4.72 KB, patch)
2010-12-06 12:31 PST, Neil Deakin
bugs: review+
Details | Diff | Splinter Review
1.9.2 patch (4.76 KB, patch)
2011-01-18 12:13 PST, Neil Deakin
christian: approval1.9.2.14+
Details | Diff | Splinter Review

Description Manuel Reimer 2010-11-01 03:43:52 PDT
So far (Firefox 3.6), RDF-rule-generated toolbar items just were left in an "empty" state and the datasource was removed, when toolbar-customize-mode was left.

With the latest Firefox trunk, RDF-rule-generated toolbar items are cloned including all children. If I just reconnect datasource after toolbar customization and call "builder.rebuild()", then all items are duplicated. One instance which is *not* controlled by the builder and one instance controlled by the builder. I have to workaround this by running over all children and removing them.

Expected behaviour: A RDF-rule generated toolbar item should either be placed in functional state (with datasource connected) or empty as in Firefox 3.6.

To reproduce, install PrefBar (addons.mozilla.org), go to toolbar customize mode, leave toolbar customize mode and then have a look at PrefBar. I've linked to the PrefBar bug, caused by this one, via URL-field of this bug.
Comment 1 Henrik Skupin (:whimboo) 2010-11-01 05:56:03 PDT
Manuel, it would be good to know when it has been regressed. CC'ing Dao who probably knows a bug which could have regressed it.
Comment 2 Sascha Grage 2010-11-01 07:03:32 PDT
Firefox:
 last good nightly: 20101026 (Windows NT 6.1; WOW64; rv:2.0b8pre)
 first bad nightly: 20101028 (Windows NT 6.1; WOW64; rv:2.0b8pre)
Comment 3 Henrik Skupin (:whimboo) 2010-11-01 07:13:14 PDT
Thanks, can you also please tell us the changeset id in about:buildconfig for both of those builds?
Comment 4 Sascha Grage 2010-11-01 07:29:50 PDT
good d253c44465ae
bad  fe4898e97431
Comment 5 Henrik Skupin (:whimboo) 2010-11-01 07:33:21 PDT
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d253c44465ae&tochange=fe4898e97431

Hm, there is not clearly an obvious bug, but could it be a regression from the Tracemonkey merge?
Comment 6 Dietrich Ayala (:dietrich) 2010-11-04 00:51:02 PDT
Blocking for investigation.
Comment 7 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2010-11-06 13:51:07 PDT
Backing out the checkin of http://hg.mozilla.org/mozilla-central/rev/3e08f8844f87 fixes this bug.
Comment 8 Henrik Skupin (:whimboo) 2010-11-06 17:04:51 PDT
(In reply to comment #7)
> Backing out the checkin of
> http://hg.mozilla.org/mozilla-central/rev/3e08f8844f87 fixes this bug.

CC'ing Neil, who wrote this patch.
Comment 9 Neil Deakin 2010-11-24 08:02:50 PST
(In reply to comment #0)
> With the latest Firefox trunk, RDF-rule-generated toolbar items are cloned
> including all children. If I just reconnect datasource after toolbar
> customization and call "builder.rebuild()", then all items are duplicated. One
> instance which is *not* controlled by the builder and one instance controlled
> by the builder. I have to workaround this by running over all children and
> removing them.
> 

Could you describe where in the code this is happening?

Or, better, just provide a short block of code which shows what you are doing to create this bug?
Comment 10 Daniel Veditz [:dveditz] 2010-12-01 16:19:03 PST
Does this happen on 3.6.13pre as well? AFAIK we took the same patch there.
Comment 11 [:Cww] 2010-12-01 17:29:42 PST
(In reply to comment #10)
Installed prefbar in a clean profile, can confirm it happens with the 3.6.13 build 2 candidate.
Comment 12 Daniel Veditz [:dveditz] 2010-12-03 10:57:17 PST
The regressing bug was backed out of the 1.9.2 branch
Comment 13 Manuel Reimer 2010-12-04 05:19:45 PST
The code, affected by this bug, starts here:
http://git.tuxfamily.org/?p=gitroot/prefbar/main.git;a=blob;f=content/prefbarOverlay.js;h=95e4b7766ea13f1d786ed220d2870053fec16d75;hb=HEAD#l201

But anything, I do here, is to reconnect the datasource.

I think the following is happening: When customizing the toolbar, Firefox moves the toolbar items around. So far this caused the datasource to be dropped. Means, that rule-generated menus or toolbaritems were just empty. By reconnecting the datasource and calling "builder.rebuild()" anything worked again.

With this regression bug, the "moving" of the items still disconnects the datasource, but the items are not longer dropped. Instead of empty menus/toolbaritems, they are now still filled. When reconnecting the datasource and calling "builder.rebuild()", the items are now duplicated. One old instance and one new instance, created by the ruleset.
Comment 14 Manuel Reimer 2010-12-04 05:20:35 PST
> The regressing bug was backed out of the 1.9.2 branch

So this means: No fix for Firefox 4?
Comment 15 christian 2010-12-06 00:08:22 PST
(In reply to comment #14)
> > The regressing bug was backed out of the 1.9.2 branch
> 
> So this means: No fix for Firefox 4?

No. It is saying we were going to take a different fix in 3.6.13 but noticed that it caused this bug. Rather than inflicting this bug on all 3.6 users, we backed out the other fix.

This will likely be worked on for FF4, as the original patch that caused this issue is still in the 4.0 repository (I believe).
Comment 16 Neil Deakin 2010-12-06 12:31:42 PST
Created attachment 495586 [details] [diff] [review]
Clear mRoot after calling Uninit
Comment 18 Henrik Skupin (:whimboo) 2010-12-12 20:52:28 PST
Manual, could you please verify that this bug is now fixed for you? Thanks.
Comment 19 Sascha Grage 2010-12-13 06:45:38 PST
yes, bug is fixed in firefox trunk build 20101213.
Comment 20 Henrik Skupin (:whimboo) 2010-12-13 17:01:34 PST
Thanks Sascha! Marking as verified fixed based on comment 19.
Comment 21 Neil Deakin 2011-01-18 12:13:07 PST
Created attachment 504810 [details] [diff] [review]
1.9.2 patch
Comment 22 christian 2011-01-18 12:46:35 PST
Comment on attachment 504810 [details] [diff] [review]
1.9.2 patch

a=LegNeato for 1.9.2.14
Comment 23 Daniel Veditz [:dveditz] 2011-01-21 11:56:46 PST
on Jan 18:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/34468ee8fdf2
Comment 24 Daniel Veditz [:dveditz] 2011-01-21 13:24:38 PST
Patch applied and seemed to fix bug on 1.9.1 branch, checked in (minus test)
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/bc91b067f42f
Comment 25 Al Billings [:abillings] 2011-01-21 15:33:13 PST
(In reply to comment #22)
> Comment on attachment 504810 [details] [diff] [review]
> 1.9.2 patch
> 
> a=LegNeato for 1.9.2.14

I don't see this in 1.9.2.13 with prefbar. I read above that we backed out the bug that caused this regression in 1.9.2. Does this bug reproduce on a released build?
Comment 26 christian 2011-01-21 15:34:34 PST
No, it only happens when bug 596232 has landed, which will be in the next version and isn't in a released version. This would be a regression if we shipped bug 596232 alone w/o fixing this.
Comment 27 Al Billings [:abillings] 2011-01-24 15:26:49 PST
I'll mark it verified as I'm not seeing the bug in nightly builds then.

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