1116027 - Don't offer "Switch to Tab" in Recent Tabs panel

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Vivek Balakrishnan[:vivek]]

This is a regression due to bug 732752.

Current: A new tab is always opened to restore the recently closed tab history even where there exists a tab with same session history.

Expected: A new tab must be spanned only if there are no tabs with same session history.

Comment 1

[Vivek Balakrishnan[:vivek]]

NI Richard and Nick for their opinion on the same as this is relevant to bug 1108426

Comment 2

[Vivek Balakrishnan[:vivek]]

Attachment: switch_to_closed_tab.patch

A new proposed patch that:
* iterates through all open tabs and compares the tab history with recent tab history
* spans a new tab to restore the closed tab iff there exists no tab open with matching session history.
* As a minor improvement, switches to the most recently closed tab when "open all tabs" option is selected.

Comment 3

[Richard Newman [:rnewman]]

I'm going to go out on a limb here and make a chain of assertions.

1. Usually a user won't have two tabs open with the exact same URL history. This assertion is even stronger when you generalize to the full session history (e.g., entered text in forms, scroll position, …).

I feel that the previous switch-to-tab behavior of Recent Tabs was an accidental/serendipitous result of not storing enough information, rather than being desirable.

2. As such, this is very much a corner case, and if anything it'll become an increasingly small corner. Not worth fixing, IMO.

3. Indeed, is it actually a bad thing that we're opening Recent Tabs in a new tab? How many users does it harm?

4. Even if a user has the same page open already, that they're in Recent Tabs implies that they want to reopen the closed tab, not switch to the one they left open. (If they wanted to do that, wouldn't they be in the tabs tray?)

That is, if I open two tabs to file bugs in Bugzilla, and I accidentally close one, my desired behavior is *not* to switch tabs; that'll be confounding, probably lose some data, and will annoy me.

Every operation with Recently Closed Tabs should open a new tab, restore the session history, and remove the entry from the list.

This is how desktop versions of both Firefox and Chrome work. Switch-to-tab is an operation in a navigational context (e.g., awesomebar), not a session-management context.

Those four assertions -- this is a very small and shrinking corner, and the new behavior is actually better -- lead me to propose: this isn't really a regression, and we should WONTFIX this bug. If anything, take the time to clean up any hooks that exist in the Recent Tabs panel.

Comment 4

[:Margaret Leibovic]

I agree with rnewman that we shouldn't have switch-to-tab for the recent tabs panel. We never made an active decision to include that UI, but we ended up with it because of the way we share logic between panels.

However, we can't just WONTFIX this bug. We still need to make sure we never show the "switch to tab" UI in the items in the recent tabs list.

vivek, let me know if you need help figuring out how to do this.

Comment 5

[:Margaret Leibovic]

Comment on attachment 8541956 [details] [diff] [review]
switch_to_closed_tab.patch

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

For the reasons rnewman stated, I don't think we should go ahead with this patch.

Comment 6

[Vivek Balakrishnan[:vivek]]

Attachment: 1116027.patch

Property added to TwoLinePageRow to override "Switch to tab" in Recent Tabs panel

Comment 7

[Vivek Balakrishnan[:vivek]]

Attachment: 1116027.patch

Reused the existing mShowIcons to suppress "switch to tabs".

Comment 8

[Vivek Balakrishnan[:vivek]]

Try server run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9469f3136cbc

Comment 9

[:Margaret Leibovic]

Comment on attachment 8542251 [details] [diff] [review]
1116027.patch

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

Looks good. Can you file a follow-up bug to rename mShowIcons to something more intuitive? If you like you could make it a [good first bug] and make yourself the mentor, and I can just come in for a final review :)

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/4278e53a8a47

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4278e53a8a47

Comment 12

[Teodora Vermesan (:TeoVermesan)]

"Switch to Tab" is not present anymore in the "Recent Tabs" panel, so:
Verified as fixed on:
Device: Alcatel One Touch
OS: Android 4.1.2
Build: Firefox for Android 37.0a1 (2015-01-04)

Comment 13

[Cristina Madaras, QA [:CristinaM]]

I will mark this as Verified fixed based on Teodora's comment 12.