Closed Bug 608687 Opened 9 years ago Closed 9 years ago

<rule>-generated toolbar items are cloned when leaving toolbar customize

Categories

(Toolkit :: Toolbars and Toolbar Customization, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- betaN+
blocking1.9.2 --- .14+
status1.9.2 --- .14-fixed
blocking1.9.1 --- .17+
status1.9.1 --- .17-fixed

People

(Reporter: Manuel.Spam, Assigned: enndeakin)

References

()

Details

(Keywords: regression, verified1.9.1, verified1.9.2)

Attachments

(2 files)

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.
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.
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)
Thanks, can you also please tell us the changeset id in about:buildconfig for both of those builds?
good d253c44465ae
bad  fe4898e97431
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?
blocking2.0: --- → ?
Blocking for investigation.
blocking2.0: ? → betaN+
(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.
(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?
Does this happen on 3.6.13pre as well? AFAIK we took the same patch there.
blocking1.9.2: --- → ?
status1.9.2: --- → ?
Blocks: 596232
(In reply to comment #10)
Installed prefbar in a clean profile, can confirm it happens with the 3.6.13 build 2 candidate.
The regressing bug was backed out of the 1.9.2 branch
blocking1.9.2: ? → ---
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.
> The regressing bug was backed out of the 1.9.2 branch

So this means: No fix for Firefox 4?
(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).
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #495586 - Flags: review?(Olli.Pettay)
Attachment #495586 - Flags: review?(Olli.Pettay) → review+
http://hg.mozilla.org/mozilla-central/rev/d87be167b410
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
blocking1.9.1: --- → .17+
blocking1.9.2: --- → .14+
Manual, could you please verify that this bug is now fixed for you? Thanks.
Flags: in-litmus-
Target Milestone: --- → mozilla2.0b8
yes, bug is fixed in firefox trunk build 20101213.
Thanks Sascha! Marking as verified fixed based on comment 19.
Status: RESOLVED → VERIFIED
Attached patch 1.9.2 patchSplinter Review
Attachment #504810 - Flags: approval1.9.2.14?
Comment on attachment 504810 [details] [diff] [review]
1.9.2 patch

a=LegNeato for 1.9.2.14
Attachment #504810 - Flags: approval1.9.2.14? → approval1.9.2.14+
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
(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?
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.
I'll mark it verified as I'm not seeing the bug in nightly builds then.
You need to log in before you can comment on or make changes to this bug.