858597 - Make switching to and from about:customizing allow you to enter and exit customization mode

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

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

The tab-strip is currently part of the group of things that are (visually speaking) available to be customized.

At the moment, the tab-strip can still be used - the background tabs are hidden behind a gradient, but they're still very much clickable.

UX would like to try making it so that clicking on a background tab exits you from customization mode, and clicking on the about:customizing tab (which would still be open) will re-enter you into that mode.

Comment 1

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

UX has requested we attempt to squeeze this into M2 if we have the chance.

Comment 2

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

Attachment: Patch v1

Hey Jared,

This seems to do the job. Care to take a peek?

-Mike

Comment 3

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

Comment on attachment 735446 [details] [diff] [review]
Patch v1

Er, n/m - this breaks things horribly if entering customization mode via the panel.

Comment 4

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

Attachment: Patch v1.1

Ok, this is better. There's a bug where if we have a about:customization tab open, and we open a new tab, and type in about:customization and press enter, the customization mode gets into a funny state (doubled items in the palette, inability to exit the mode).

I think I'd like to fix that in a follow-up, though.

Comment 5

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

Comment on attachment 735450 [details] [diff] [review]
Patch v1.1

I think I've found a solution to that bug I mentioned in my last comment. Give me a second to try it...

Comment 6

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

Attachment: Patch v1.2

Alright, this should take care of this bug now (adds a rule such that we don't need to switch tabs, or create an about:customizing tab if the tab we're currently on is already at about:customizing).

As I mentioned to jaws in meatspace, I'm a tad worried that this patch could result in accidental tab-tearing when exiting customization mode via a tab switch... UX should give this a shot when it lands and let me know what they think.

Comment 7

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

Comment on attachment 735499 [details] [diff] [review]
Patch v1.2

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

::: browser/modules/CustomizeMode.jsm
@@ +196,5 @@
> +      let customizationTab = this.browser.selectedTab;
> +      if (this.browser.browsers.length == 1) {
> +        this.window.BrowserOpenTab();
> +      }
> +      this.browser.removeTab(customizationTab);

Most of the patch looks fine but as we talked about in person, we should probably not close the tab if the history of the tab is greater than zero. In that case, I think we should just navigate to the page previous to the customization.

Comment 8

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

Attachment: Patch v1.3

Now using the history as an indicator for whether or not exiting customization mode should cause the browser to go back, or should close.

Comment 9

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

Comment on attachment 735554 [details] [diff] [review]
Patch v1.3

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

::: browser/modules/CustomizeMode.jsm
@@ +689,5 @@
> +   * Looks at the currently selected browser tab, and if the location
> +   * is set to kAboutURI and we're not customizing, enters customize mode.
> +   * If we're not at kAboutURI and we are customizing, exits customize mode.
> +   */
> +  _considerBrowser: function() {

Hm, I think we can find a better name for this function. Perhaps `toggleCustomizatoinModeIfNecessary()`?

Comment 10

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

Attachment: Patch v1.4 (r+'d by jaws)

Yep, I dig the function name change. Thanks for the review.

Comment 11

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

Landed in jamun as https://hg.mozilla.org/projects/jamun/rev/5805b8e97a98

Comment 12

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

Landed on UX as https://hg.mozilla.org/projects/ux/rev/5805b8e97a98

Comment 13

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/933d587cfd04