Bugzilla

(In reply to Sarah Clements [:sclements] from comment #4)
> (In reply to Sam Foster [:sfoster] (he/him) from comment #3)
> > Possibly I'm missing something obvious, but how does making it `removable` help unless we also make the sidebar a widget area? We're still not be able find a customization target for it, and `isWidgetRemovable` throws [when it tries to find the node again](https://searchfox.org/mozilla-central/source/browser/components/customizableui/CustomizableUI.sys.mjs#3455-3456).  I guess I've not really understood all the CUI logic yet - I'm just trying to figure out if there was a plan here or if the removable thing was just a potential direction to explore.
> 
> This was definitely not a "we have to make it removeable", but rather a possible direction to explore that Gijs mentioned during review of the initial patch.

Yeah - I had not even gotten as far as actually investigating the stack in detail (like comment 1). Here is the entire para that led to this bug being filed:

> Yet another option would be to actually make tabbrowser-tabs removable, and then fix up whatever else is complaining in customizableUI. It likely still couldn't actually be put anywhere else by users easily because in customize mode we don't add "normal" customize mode drag/drop behaviour to the tabstrip, so you can still switch tabs - though I'm sure people would figure out ways around that using the browser console or by manipulating prefs or w/e, I expect people already do that anyway so it's probably no huge deal. This is probably the easiest option - **assuming it actually helps with the reset functionality**.

(Emphasis added. :-) )

So mostly the issue is that "restore defaults" is completely broken once this pref is turned on. We should fix that.

(In reply to Sam Foster [:sfoster] (he/him) from comment #1)
> In [the discussion on the patch](https://phabricator.services.mozilla.com/D213855) for Bug 1899346, :Gijs suggests that the simplest way forward for now might be to have the restore defaults button in the customization panel first reset back to horizontal tabs. That seems like a good first step here. It doesnt change the fact that any CUI method that calls getCustomizationTarget("tabbrowser-tabs") will end up throwing the same exception. That's a bit of a chicken and egg problem with Bug 1899346. I'm not sure if it will turn out to need to be fixed here first or there.

FWIW, in _that_ patch I was actually asking not about the tabbrowser-tabs themselves as much as I was about _the other widgets_, as the `alltabs-button` is not marked as removable (and I thought the same would be true for the new tab button, but that's already removable). Whether those widgets are removable will affect how happy CUI is with that patch moving them to other places (other toolbars or just the palette, if we don't think they should be visible in the tabstrip). When I commented, your patch left those widgets physically in the tabstrip, and I was trying to suggest that instead we could just `removeWidgetFromArea` those widgets if we made them removable.

The upshot would be that we could copy the tabstrips pre-vertical-tabs placement list to a separate pref, and for any of the default widgets we could call `removeWidgetFromArea`, and any others we could put in the palette. Then if/when the user turns off vertical tabs, we can rebuild the area with the original set of placements. That seems neater than than making the widgets permanently unavailable (esp. the new tab button, but even the all tabs button could be something users want, if their tabs do not fit in the sidebar and the scrolling is annoying - much like it can be with horizontal tabs.)

(In reply to Sarah Clements [:sclements] from comment #4)
> (In reply to Sam Foster [:sfoster] (he/him) from comment #3)
> > Possibly I'm missing something obvious, but how does making it `removable` help unless we also make the sidebar a widget area? We're still not be able find a customization target for it, and `isWidgetRemovable` throws [when it tries to find the node again](https://searchfox.org/mozilla-central/source/browser/components/customizableui/CustomizableUI.sys.mjs#3455-3456).  I guess I've not really understood all the CUI logic yet - I'm just trying to figure out if there was a plan here or if the removable thing was just a potential direction to explore.
> 
> This was definitely not a "we have to make it removeable", but rather a possible direction to explore that Gijs mentioned during review of the initial patch.

Yeah - I had not even gotten as far as actually investigating the stack in detail (like comment 1). Here is the entire para that led to this bug being filed:

> Yet another option would be to actually make tabbrowser-tabs removable, and then fix up whatever else is complaining in customizableUI. It likely still couldn't actually be put anywhere else by users easily because in customize mode we don't add "normal" customize mode drag/drop behaviour to the tabstrip, so you can still switch tabs - though I'm sure people would figure out ways around that using the browser console or by manipulating prefs or w/e, I expect people already do that anyway so it's probably no huge deal. This is probably the easiest option - **assuming it actually helps with the reset functionality**.

(Emphasis added. :-) )

So mostly the issue is that "restore defaults" is completely broken once this pref is turned on. We should fix that.

(In reply to Sam Foster [:sfoster] (he/him) from comment #1)
> In [the discussion on the patch](https://phabricator.services.mozilla.com/D213855) for Bug 1899346, :Gijs suggests that the simplest way forward for now might be to have the restore defaults button in the customization panel first reset back to horizontal tabs. That seems like a good first step here. It doesnt change the fact that any CUI method that calls getCustomizationTarget("tabbrowser-tabs") will end up throwing the same exception. That's a bit of a chicken and egg problem with Bug 1899346. I'm not sure if it will turn out to need to be fixed here first or there.

FWIW, in _that_ patch I was actually asking not about the tabbrowser-tabs themselves as much as I was about _the other widgets_, as the `alltabs-button` is not marked as removable (and I thought the same would be true for the new tab button, but that's already removable). Whether those widgets are removable will affect how happy CUI is with that patch moving them to other places (other toolbars or just the palette, if we don't think they should be visible in the tabstrip). When I commented, your patch left those widgets physically in the tabstrip, and I was trying to suggest that instead we could just `removeWidgetFromArea` those widgets if we made them removable.

The upshot would be that we could copy the tabstrips pre-vertical-tabs placement list to a separate pref, and for any of the default widgets we could call `removeWidgetFromArea`, and any others we could put in the toolbar. Then if/when the user turns off vertical tabs, we can rebuild the area with the original set of placements. That seems neater than than making the widgets permanently unavailable (esp. the new tab button, but even the all tabs button could be something users want, if their tabs do not fit in the sidebar and the scrolling is annoying - much like it can be with horizontal tabs.)