877524 - Add comment to browser.xul about default placements arrays

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

The relationship between the nodes in the toolbars and their defaultset's have changed. It used to be when a developer added a new default toolbar button to a toolbar, they needed to update the defaultset attribute of that toolbar as well.

That's changing with the new CustomizableUI module. Each toolbar has their own default placements defined in there, and we should add a comment to browser.xul to mention this for devs who might not be familiar with CustomizableUI.jsm.

Comment 1

[:Gijs (he/him)]

Attachment: Add comment

I'm guessing you mean this.

In this case, shouldn't we get rid of the defaultset attribute, too, to avoid confusion?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 755878 [details] [diff] [review]
Add comment

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

I have no problem with removing defaultset. Can you please file a (non-Australis-blocking) bug for that?

Thanks for the comment!

::: browser/base/content/browser.xul
@@ +500,5 @@
>  
> +    <!--
> +           CAVEAT EMPTOR
> +           Should you need to add items to the toolbar here, make sure to also add them
> +           to the default sets of buttons in CustomizableUI.jsm, so the customization code

let's use placements here instead of "sets", since that seems to be the nomenclature within CustomizableUI.jsm.

Comment 3

[:Gijs (he/him)]

Pushed, filed bug 877856 as a followup.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Please include the changeset URL when you push :)

https://hg.mozilla.org/projects/ux/rev/4484d3d0525b

Comment 5

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/4484d3d0525b