Closed Bug 945753 Opened 7 years ago Closed 7 years ago

Calling insertItem on a toolbar causes any non-displayed toolbar items to be removed from "currentset" attribute.

Categories

(Firefox :: Toolbars and Customization, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: morac, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [Australis:P3])

Attachments

(1 file)

When calling the insertItem on the Toolbar in Firefox 28, it clears out any item not currently in the toolbar from the "currentset" attribute of that toolbar.  It still persists since if I open a new window the item gets added back to the "currentset" attribute of the toolbar in that window.

Since my Session Manager bootstrap add-on can add two items to the toolbar, I now need to collect the location of both items from "currentset" prior to installing any of them.  If I don't and both items are in the same toolbar, then when I add the first one, the second one gets removed from "currentset" in that toolbar.  When my addon goes to find where the next item should go, it won't be able to find it.  Since the item is still in the toolbar as far as Firefox is concerned, the item is effectively lost at that point since it won't show up in the Customize tab either.

As stated, this does not affect the persisted state, so if I remove the first item and then open a new window, the second item will show up since it is now found in the "currentset".

I currently work around this in my Session Manager addon, by collecting the locations of both toolbar items before trying to add either of them.  This could cause issues for other add-ons if they look at the "currentset" attribute.  I did not check the currentSet JS object to see if that was correct.


On a side note, the reason I use my own method of adding toolbar items in a bootstrap add-on instead of using the SDK is because of bug 854226, which wasn't fixed until very recently.
Patch incoming.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Comment on attachment 8341758 [details] [diff] [review]
should update currentset attribute with placements, not currentSet property, to avoid nixing non-existant items ourselves,

Review of attachment 8341758 [details] [diff] [review]:
-----------------------------------------------------------------

Note to self: UPDATE THE DESCRIPTION. Because no Australis == sad Holly.
Whiteboard: [Australis:P3]
remote:   https://hg.mozilla.org/integration/fx-team/rev/35a8b1df1374
Whiteboard: [Australis:P3] → [Australis:P3][fixed-in-fx-team]
Blegh. I screwed up:

remote:   https://hg.mozilla.org/integration/fx-team/rev/ff4388261db5


     1.7      let container = aAreaNode.customizationTarget;
     1.8      let [insertionContainer, nextNode] = this.findInsertionPoints(widgetNode, aNextNodeId, aAreaNode);
     1.9      this.insertWidgetBefore(widgetNode, nextNode, insertionContainer, areaId);
    1.10  
    1.11      if (gAreas.get(areaId).get("type") == CustomizableUI.TYPE_TOOLBAR) {
    1.12 -      areaNode.setAttribute("currentset", gPlacements.get(areaId).join(','));
    1.13 +      aAreaNode.setAttribute("currentset", gPlacements.get(areaId).join(','));
(In reply to :Gijs Kruitbosch from comment #5)
> Blegh. I screwed up:
> 
> remote:   https://hg.mozilla.org/integration/fx-team/rev/ff4388261db5
> 
> 
>      1.7      let container = aAreaNode.customizationTarget;
>      1.8      let [insertionContainer, nextNode] =
> this.findInsertionPoints(widgetNode, aNextNodeId, aAreaNode);
>      1.9      this.insertWidgetBefore(widgetNode, nextNode,
> insertionContainer, areaId);
>     1.10  
>     1.11      if (gAreas.get(areaId).get("type") ==
> CustomizableUI.TYPE_TOOLBAR) {
>     1.12 -      areaNode.setAttribute("currentset",
> gPlacements.get(areaId).join(','));
>     1.13 +      aAreaNode.setAttribute("currentset",
> gPlacements.get(areaId).join(','));

Gah, that's a screwup on my part too. Sorry that slipped through.
https://hg.mozilla.org/mozilla-central/rev/35a8b1df1374
https://hg.mozilla.org/mozilla-central/rev/ff4388261db5
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P3][fixed-in-fx-team] → [Australis:P3]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.