1003588 - Separator in customize menu creates a blank (and unremovable) space

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Jackson Sul]

Attachment: separator in customize menu.png

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140428194004

Steps to reproduce:

Move a separator from the navigation bar to the menu that opens when you hit the menu button (the one with the three-lines as an icon). 




Actual results:

It created a blank space, offset the line, and now cannot be moved. 

See attached picture: the separator is to the right of the home button.


Expected results:

Since adding a separator to the navigation bar no longer seems possible (I think?), I'm not really sure. I suppose the separator should be visible, not just show a blank space, and it certainly should be movable.

Comment 1

[Jamie Bainbridge]

Confirm this still exists in F29 final release.

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:29.0) Gecko/20100101 Firefox/29.0
Build ID: 20140428120048

Comment 2

[Cornel Ionce [:noni] [Hubs QA]]

Marking new based on comment 1.

Comment 3

[:Gijs (he/him)]

Attachment: don't let spacers, springs and separators go into the panel,

Unfortunately, I don't think making these items behave well in the menupanel is something that we should spend a significant amount of time on. Instead, I think we should just not let people move them there; they're only really useful in toolbars.

Comment 4

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

Comment on attachment 8420342 [details] [diff] [review]
don't let spacers, springs and separators go into the panel,

I can't reproduce this bug pre-patch. I used DOM Inspector to add a <xul:toolbarseparator> to the #nav-bar. I then entered customization mode, but I can't drag the separator anywhere.

Aibara, do you see the normal drag icon when hovering over a separator in your toolbar?

Comment 5

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

Comment on attachment 8420342 [details] [diff] [review]
don't let spacers, springs and separators go into the panel,

In my reproduction attempts I was adding the separator without making it removable, nor tracked by CustomizableUI.

This patch works find. Thanks!

Comment 6

[:Gijs (he/him)]

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

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/659bb081385b

Comment 8

[:Gijs (he/him)]

Comment on attachment 8420342 [details] [diff] [review]
don't let spacers, springs and separators go into the panel,

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Users can put spacers/springs/separators in the menu panel, and not remove them
Testing completed (on m-c, etc.): local, on m-c, has automated test
Risk to taking this patch (and alternatives if risky): medium to low. I don't expect anything to break here, but equally I don't think the severity * frequency of this is high enough to warrant the risk to uplift to beta in the last 3 weeks of the cycle.
String or IDL/UUID changes made by this patch: none

Comment 9

[Cornel Ionce [:noni] [Hubs QA]]

Mozilla/5.0 (X11; Linux i686; rv:30.0) Gecko/20100101 Firefox/30.0

The user is no longer able to place separators inside the Menu panel on latest Nightly, build ID: 20140514030203.
Marking issue verified.

Comment 10

[Cornel Ionce [:noni] [Hubs QA]]

Mozilla/5.0 (X11; Linux i686; rv:32.0) Gecko/20100101 Firefox/32.0

Oups, wrong User Agent pasted above.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/029ad42a9d6c

Comment 12

[Jamie Bainbridge]

For anyone hitting this now, you can remove the separator manually:

* Stop Firefox
* Edit the file: ~/.mozilla/firefox/<profile directory>/prefs.js
* Find the line which starts: user_pref("browser.uiCustomization.state"
* Remove the item: \"customizableui-special-separator2\",
* Start Firefox

Comment 13

[GiTcode]

This causes a problem for several extensions that add support for "special widgets" to be added to (among other places) the menu panel.

Including: 
Classic Theme Restorer
The Puzzle Piece
And mine, The Addon Bar (Restored).
I have worked around it in the current beta version, (source here: https://addons.mozilla.org/en-US/firefox/files/browse/260456/file/modules/spacers.js#top), but it is quite hackish.

Could we add a way to let addons add this functionality cleanly?

Comment 14

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

(In reply to GiTcode from comment #13)
> This causes a problem for several extensions that add support for "special
> widgets" to be added to (among other places) the menu panel.
> 
> Including: 
> Classic Theme Restorer
> The Puzzle Piece
> And mine, The Addon Bar (Restored).
> I have worked around it in the current beta version, (source here:
> https://addons.mozilla.org/en-US/firefox/files/browse/260456/file/modules/
> spacers.js#top), but it is quite hackish.
> 
> Could we add a way to let addons add this functionality cleanly?

Not really. The basic premise behind these add-ons adding capabilities that aren't supported by default means that these add-ons should go the full distance and also add the necessary CSS/styling to make placements in these areas look nice and fit in.

Comment 15

[GiTcode]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #14)
> Not really. The basic premise behind these add-ons adding capabilities that
> aren't supported by default means that these add-ons should go the full
> distance and also add the necessary CSS/styling to make placements in these
> areas look nice and fit in.

Right, I agree that the addons should add the proper styling and such to make them fit in, but I was referring to the (as of this bug landing) great difficulty in getting the special widgets into the menu in the first place.  

Perhaps we could add a simple switch that an addon could use to let the browser know that special widgets can be in the menu.  Then on disable or uninstall the addon could turn the switch back off.