791044 - [Metro] It should be possible to close the last tab

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: General, defect)

Description

[Matt Brubeck (:mbrubeck)]

Attachment: patch

In desktop Firefox, closing the last tab (e.g. with Control-W) closes the browser window.  We should allow this in Metro too.

(We didn't allow users to close the last tab in Fennec, partly because long shutdown and startup times made it very expensive to get back the browser after doing this -- but that may change too (bug 747633).)

Comment 1

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

I have an different opinion on this: 
My suggestion on closing last tab is to keep the last tab open unless users swipe top down to close it. If the last tab is a certain website, tapping the close btn should change it back to the Start Page content.

From a strategical perspective,
On mobile device, users are given limited choices because of limited screen size. users can't see any other current active apps unless they choose to quit the current one. Even though W8 is also made for desktop users, Metro UI is still optimized for mobile usage. Using snap view, only 2 apps could be seen the same time. 
So it's important to capture users' attention on a mobile device, otherwise we could lose them very easily. Especially for tasks like content browsing and discovery, there are many mobile apps, such as dedicated for those things. This is very different from desktop world where browsers are still the main entrance for discovery and browsing.

So, I feel Firefox on a mobile device, should not drive the valuable attention away , by closing the application itself. It's highly possible that those users who left might not come back.


From UX perspective: 
When user taps/clicks to close the last tab, it doesn't equal to user feeling like quitting the browser. It's highly possible that they just don't want to keep browsing on this current site. They might still be interested in discovering other things if Firefox gives them good suggestions. 

As far as I feel, auto-close is not common for mobile apps. Because it's very easy and natural to close an app, by swiping down(W8)/tapping home btn. So my concern would be, users might think Firefox just crashed if they saw things close by itself.

Lastly, on Metro UI, I believe there is no indication on whether the app is active or not. Unlike desktop there is the dock that contains all active softwares. Even though the last window is closed, users can still tell that Firefox is open in bg. But on Metro UI, there is not launch pad, or task manager, to indicate those things. So we should be careful closing the application by itself. 

Please let me know if this makes sense to you. Open to discussion.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 660913 [details] [diff] [review]
patch

>diff --git a/browser/metro/base/content/browser.js b/browser/metro/base/content/browser.js

>   _doCloseTab: function _doCloseTab(aTab) {
>     let nextTab = this._getNextTab(aTab);
>     if (!nextTab)
>-       return;
>+       this.quit();

quit might not actually close the window. There are a few ways it could be aborted. You could check window.closed too see if we are closing the window.

If we are not closing the window, what should we do? I think we should bail, like we currently do and abort the tab close.

Let's figure out the UX part first.

Comment 3

[Jim Mathies [:jimm]]

(In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #1)
> I have an different opinion on this: 
> My suggestion on closing last tab is to keep the last tab open unless users
> swipe top down to close it. If the last tab is a certain website, tapping
> the close btn should change it back to the Start Page content.

This is what I expect when I close that last tab. I'm definitely not trying to close the browser - I'm just trying to clear the tab off the tab bar.

Comment 4

[Jim Mathies [:jimm]]

Attachment: patch

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 664497 [details] [diff] [review]
patch

>diff --git a/browser/metro/base/content/browser.js b/browser/metro/base/content/browser.js

>   closeTab: function closeTab(aTab, aOptions) {

>+    // If this is the last tab, load the home page and display
>+    if (!this._getNextTab(tab)) {
>+      tab.openURI(Browser.getHomePage());
>+      return;
>+    }

Two things:
1. Browser.loadURI should already handle what tab.openURI is doing, so just use it.
2. I don't think just loading the startpage into the existing tab is the "right thing" to do. I'm not sure the user wants to be able to go "back" and see the session history. I think we want to add a new tab, loaded with the homepage, and kill the previous tab altogether.

Make sense?

Comment 6

[Jim Mathies [:jimm]]

Attachment: patch v.2

I don't think loadURI opens new tabs anymore, it just loads into selectedBrowser. Rather than put this logic in Browser though I put it in BrowserUI which I think is more appropriate.

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 664598 [details] [diff] [review]
patch v.2

>   closeTab: function closeTab(aTab) {
>-    // If no tab is passed in, assume the current tab
>-    Browser.closeTab(aTab || Browser.selectedTab);
>+    // If we only have one tab, open a new one
>+    if (aTab && Browser.tabs.length == 1) {
>+      Browser.addTab();
>+      Browser.closeTab(aTab);
>+    } else {
>+      // If no tab is passed in, assume the current tab
>+      Browser.closeTab(aTab || Browser.selectedTab);
>+    }
>   },

What if no tab is passed in and there's only one tab left?

Comment 8

[Jim Mathies [:jimm]]

(In reply to Dão Gottwald [:dao] from comment #7)
> Comment on attachment 664598 [details] [diff] [review]
> patch v.2
> 
> >   closeTab: function closeTab(aTab) {
> >-    // If no tab is passed in, assume the current tab
> >-    Browser.closeTab(aTab || Browser.selectedTab);
> >+    // If we only have one tab, open a new one
> >+    if (aTab && Browser.tabs.length == 1) {
> >+      Browser.addTab();
> >+      Browser.closeTab(aTab);
> >+    } else {
> >+      // If no tab is passed in, assume the current tab
> >+      Browser.closeTab(aTab || Browser.selectedTab);
> >+    }
> >   },
> 
> What if no tab is passed in and there's only one tab left?

Nothing happens, which is the current behavior. Since we are the callers of closeTab it shouldn't ever happen.

Comment 9

[Dão Gottwald [:dao]]

Maybe it doesn't happen right now, but in order to keep the API sane surely BrowserUI.closeTab using the selected tab when no tab was passed in shouldn't affect whether a new tab is added.

Comment 10

[Jim Mathies [:jimm]]

(In reply to Dão Gottwald [:dao] from comment #9)
> Maybe it doesn't happen right now, but in order to keep the API sane surely
> BrowserUI.closeTab using the selected tab when no tab was passed in
> shouldn't affect whether a new tab is added.

Ok, so how bout something like this:

closeTab: function closeTab(aTab) {
  // If no tab is passed in, assume the current tab
  if (aTab == undefined || !aTab)
    aTab = Browser.selectedTab;

  // If we only have one tab, open a new one
  if (Browser.tabs.length == 1) {
    Browser.addTab();
  }
  Browser.closeTab(aTab);
},

Comment 11

[Dão Gottwald [:dao]]

That will work. The comparison with undefined is redundant, "if (!aTab)" is sufficient.

Comment 12

[Jim Mathies [:jimm]]

Attachment: patch v.3

Comment 13

[Jim Mathies [:jimm]]

(In reply to Jim Mathies [:jimm] from comment #12)
> Created attachment 664633 [details] [diff] [review]
> patch v.3

I guess I can get rid of the brackets on the second if statement as well.

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 664633 [details] [diff] [review]
patch v.3

The indentation in browser-ui.js is messed up. Also, on second thought it seems like you could simplify that part of the patch to this:

   closeTab: function closeTab(aTab) {
+    // If we only have one tab, open a new one
+    if (Browser.tabs.length == 1) {
+      Browser.addTab();

     // If no tab is passed in, assume the current tab
     Browser.closeTab(aTab || Browser.selectedTab);
   },

Comment 15

[Dão Gottwald [:dao]]

(In reply to Dão Gottwald [:dao] from comment #14)
> seems like you could simplify that part of the patch to this:
> 
>    closeTab: function closeTab(aTab) {
> +    // If we only have one tab, open a new one
> +    if (Browser.tabs.length == 1) {
> +      Browser.addTab();

And make sure you don't mess up the brackets like I just did...

Comment 16

[Jim Mathies [:jimm]]

Attachment: patch v.4

Ok, here's the latest.

Comment 17

[Dão Gottwald [:dao]]

Comment on attachment 664666 [details] [diff] [review]
patch v.4

>   closeTab: function closeTab(aTab) {
>-    // If no tab is passed in, assume the current tab
...
>+    // If no tab is passed in, assume the current tab

What's going on here? DOS line endings?

>   addTab: function browser_addTab(aURI, aBringFront, aOwner, aParams) {
>+    let uri = aURI || this.getHomePage();
>     let params = aParams || {};
>-    let newTab = new Tab(aURI, params);
>+    let newTab = new Tab(uri, params);

This seems reasonable, but I'll note that it's inconsistent with the tabbed browser API of desktop Firefox, which just adds a blank tab if you don't pass an URI.

Comment 18

[Jim Mathies [:jimm]]

Attachment: patch v.5

I removed the code from Browser per your comments, and cleaned up the Windows line endings in BrowserUI.

Comment 19

[Jim Mathies [:jimm]]

\o/ thanks for your patience!

Comment 20

[rospkos.07]

I hv another view on the interaction flow of closing a tab 

The user would press long on a tab in tab bar. the time could be between 2 sec and 3 sec. Then the animation of crossing the tab would appear on the pressed tab, presenting the ongoing action that is the tab is being closed. 

the top down action would be for closing the firefox broswer .

Comment 21

[Matt Brubeck (:mbrubeck)]

Resolving bugs in the Firefox for Metro product that are fixed on the elm branch.  Sorry for the bugspam.  Search your email for "bugspam-elm" if you want to find and delete all of these messages at once.